-
Notifications
You must be signed in to change notification settings - Fork 53
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
Create speaker account with SSO as part of the answer to Call for Proposals #508
base: development
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request implements Single Sign-On (SSO) for speaker accounts, allowing users to log in via external providers and redirecting them to the correct page after authentication. It also refactors the login views to use class-based views. Sequence diagram for SSO login flow with redirectsequenceDiagram
actor User
participant Browser
participant OAuthLoginView
participant OAuthReturnView
participant ExternalProvider
participant Database
User->>Browser: Click 'Login with SSO'
Browser->>OAuthLoginView: GET /oauth_login/{provider}/ with next URL
OAuthLoginView->>OAuthLoginView: set_oauth2_params()
OAuthLoginView->>ExternalProvider: Redirect to provider login
ExternalProvider->>OAuthReturnView: Return with auth data
OAuthReturnView->>Database: get_or_create_user()
OAuthReturnView->>OAuthReturnView: prepare_oauth2_params()
OAuthReturnView->>Browser: Redirect to original page
Browser->>User: Show authenticated page
Class diagram for updated OAuth viewsclassDiagram
class View {
<<Django>>
}
class OAuthLoginView {
+get(request, provider)
-set_oauth2_params(request)
}
class OAuthReturnView {
+get(request)
-get_or_create_user(request)
-prepare_oauth2_params(oauth2_params)
}
View <|-- OAuthLoginView
View <|-- OAuthReturnView
note for OAuthLoginView "Handles initial SSO request"
note for OAuthReturnView "Processes SSO callback"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @HungNgien - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider cleaning up the oauth2_params session data in error cases as well to avoid stale data. This could be done by moving the cleanup to a finally block or similar.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…ntyay-tickets into sso_login_redirect
Make social login to redirect to previous page when Login with SSO in other components
Summary by Sourcery
New Features: