extract many auth helpers from shared to middleware - #47934
Merged
Conversation
davidsbailey
changed the title
davidsbailey
changed the title
davidsbailey
marked this pull request as ready for review
September 2, 2022 16:45
davidsbailey
commented
Sep 2, 2022
| # @returns [Boolean] true if the current user is an admin. | ||
| def admin? | ||
| current_user && !!current_user[:admin] | ||
| end |
Member
Author
There was a problem hiding this comment.
still used outside of dashboard here:
code-dot-org/pegasus/helper_modules/dashboard.rb
Lines 62 to 65 in 6e49521
Member
Author
There was a problem hiding this comment.
has_permission? just got removed in #47921, so now admin? can move to dashboard too.
pablo-code-org
approved these changes
Sep 2, 2022
This reverts commit d85b193.
davidsbailey
commented
Sep 2, 2022
| def current_user | ||
| return nil if (id = current_user_id).nil? | ||
| @dashboard_user ||= DASHBOARD_DB[:users][id: id] | ||
| end |
Member
Author
There was a problem hiding this comment.
current_user is still used in pegasus here:
since we are just checking whether the user is signed in, it looks possible to change it so that it gets that information from a cookie instead, like we do here:
code-dot-org/pegasus/sites.v3/code.org/public/index.haml
Lines 18 to 21 in dc24d3b
bethanyaconnor
approved these changes
Sep 6, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
depends on #47931 .
as part of rounding up remaining DASHBOARD_DB uses outside of the dashboard directory, I discovered that many helpers within auth_helpers.rb are now only used by dashboard/legacy, and can therefore be moved into dashboard/legacy. This kind of move will help us add rules which enforce that only dashboard code is accessing dashboard DB.
Testing story
existing unit tests appear to cover this change pretty well.