-
Notifications
You must be signed in to change notification settings - Fork 33
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
Update OEP-42 Authentication proposing consolidation of authentication code #541
Comments
Note that any exceptional custom authentication code examples should also be documented in the OEP and/or ticketed to be moved or replaced. This includes at least the two examples for XBlock handler and notes JWT decoder. |
Notes had created custom JWT authentication long ago that should be replaced. See private ticket https://2u-internal.atlassian.net/browse/TNLARCHIVE-4501 with description:
|
Is this ever going to happen? We should be moving away from writing "aspirational" OEPs that don't actually reflect either our current architecture or a new architecture that a team is actively working on |
To start, this should just be an ADR (or OEP update) stating what people should do or not do. That is not aspirational. Follow-up work to either attempt to prevent this, and/or fix any past exceptions to this rule may or may not be done, and is potentially aspirational, but is a separate issue. |
That makes sense. How would you feel about a bit of a middle ground around adding a "Best Practices" section that calls out what our ideal case is, and possibly points at places we do it right (and wrong!) I know this isn't a thing you can do today, but of all of your open issues in this repo, this one feels like the most needing of your attention - you're likely the best person in the community to address it. Would it be possible to get this on your radar, maybe July-September timeframe? I'd be happy to help copyedit / do the mechanics of the PR / be arbiter. |
|
Auth isn't on our immediate radar but I'll add @feanil in case that changes. |
Our authentication code is mostly consolidated under edx-drf-extensions and edx-platform's oauth_dispatch (as examples).
Some services use custom authentication code, such as edx-platform XBlocks code or notes custom JWT decoder. These special cases are usually discovered through breaking changes in Production.
This ticket is about codifying our wish to both consolidate authentication code in fewer locations, and to not introduce custom authentication code unless absolutely necessary. Even if the custom code is used in a single service, it is probably better to add it to the consolidated code base, because it almost certainly interacts with the rest of the authentication code.
The text was updated successfully, but these errors were encountered: