-
Notifications
You must be signed in to change notification settings - Fork 28
BB2-4250: Make v3_endpoints waffle switch app specific #1429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BB2-4250: Make v3_endpoints waffle switch app specific #1429
Conversation
…er waffle_flag. Working for read/search v3 calls, v3 auth/token flows (still need to add to some other auth views)
… flag is not enabled for that app
…s in the flag. Add 403 handling for userinfo v3
…Token and Authorization views
…ier in auth process
jimmyfagan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is close to being good to go, but just a little more fine tuning. Hopefully we can just do a quick working session on some of this in the morning.
apps/dot_ext/views/authorization.py
Outdated
| version = get_api_version_number_from_url(path_info) | ||
| # If it is not version 3, we don't need to check anything, just return | ||
| if version == Versions.V3 and self.validate_v3_call: | ||
| self.validate_v3_authorization_request() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In testing with a running local server with the flag created but without any apps enabled, I ended up with a generic 500 server error when issuing the authorize request. Seems like maybe in this path we need to add some kind of exception handling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming this was a v3 auth flow, do you know what the Everyone setting was? Had to be No or Unknown, but just want to confirm. Have not been able to reproduce the generic 500 error on the authorize request yet. I also set the superusers setting to False here, though I don't think that would have any bearing.
…emove validate_v3_call variable from AuthorizationView
|
Confirmed today that we throw 404s when the flag is there but v3_endpoints switch is not active. Might be worth another set of eyes on that as well as that is what it will be in prod. |
jimmyfagan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
JIRA Ticket:
BB2-4250
What Does This PR Do?
This PR ensures that once we enable the v3_endpoints waffle switch, we are able to control access to v3 endpoints on an app by app basis.
What Should Reviewers Watch For?
If you're reviewing this PR, please check for these things in particular:
Validation
UPDATE dot_ext_application SET user_id = {{user_id added in django admin to flag}} WHERE id = 1;UPDATE dot_ext_application SET user_id = {{any user_id besides the one added to the flag in django admin}} WHERE id = 1;What Security Implications Does This PR Have?
Please indicate if this PR does any of the following:
security engineer's approval.
Any Migrations?
We are not adding data to the database, but we will have to create a waffle flag as we roll this out. We will then add specific user_ids to that flag to enable v3 endpoint access.
etc)