-
Notifications
You must be signed in to change notification settings - Fork 55
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 #520
Create speaker account with SSO as part of the answer to Call for Proposals #520
Conversation
…ntyay-tickets into sso_login_redirect
Reviewer's Guide by SourceryThis pull request introduces the functionality to redirect users back to the original page after they log in using SSO from other components. It also includes a new schema for OAuth2 parameters. Sequence diagram for SSO login with redirect flowsequenceDiagram
actor User
participant Frontend
participant OAuthLoginView
participant OAuthReturnView
participant Provider
participant User_DB
User->>Frontend: Click Login with SSO
Frontend->>OAuthLoginView: GET /oauth_login/{provider}/ with next URL
OAuthLoginView->>OAuthLoginView: set_oauth2_params()
Note over OAuthLoginView: Store OAuth2 params in session
OAuthLoginView->>Provider: Redirect to provider login URL
Provider->>OAuthReturnView: Return with auth data
OAuthReturnView->>User_DB: get_or_create_user()
OAuthReturnView->>OAuthReturnView: process_login_and_set_cookie()
Note over OAuthReturnView: Retrieve OAuth2 params from session
OAuthReturnView->>Frontend: Redirect to original page with OAuth2 params
Class diagram for OAuth views and parametersclassDiagram
class OAuthLoginView {
+get(request: HttpRequest, provider: str): HttpResponse
-set_oauth2_params(request: HttpRequest): None
}
class OAuthReturnView {
+get(request: HttpRequest): HttpResponse
-get_or_create_user(request: HttpRequest): User
}
class OAuth2Params {
+response_type: str
+client_id: str
+redirect_uri: str
+scope: str
+state: str
}
note for OAuth2Params "New model for OAuth2 parameters"
OAuthLoginView ..> OAuth2Params : uses
OAuthReturnView ..> OAuth2Params : uses
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 and found some issues that need to be addressed.
Blocking issues:
- The URL validation should be strengthened to prevent path traversal attacks (link)
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🔴 Security: 1 blocking issue, 1 other issue
- 🟢 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.
parsed = urlparse(next_url) | ||
|
||
# Only allow relative URLs | ||
if parsed.netloc or parsed.scheme: |
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.
🚨 issue (security): The URL validation should be strengthened to prevent path traversal attacks
Consider adding checks for '../' sequences and ensuring the path starts with an allowed prefix to prevent unauthorized access to different parts of the application.
messages.error( | ||
request, _("Error while authorizing: no email address available.") | ||
) | ||
logger.error("Error while authorizing: %s", e) |
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.
🚨 issue (security): Logging raw exception could expose sensitive information
Consider logging a sanitized error message instead of the raw exception to prevent potential exposure of sensitive information in logs.
Make social login to redirect to previous page when Login with SSO in other components
Summary by Sourcery
New Features: