-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Show Username/Password Login only if not disabled #3435
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
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughA new backend API endpoint is added to expose whether user-password login is disabled. The frontend login page now conditionally displays the user-password login form and OAuth options based on this backend flag, and shows an informational message if no login methods are available. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend (Login.vue)
participant Backend API (system_settings.py)
participant System Settings
User->>Frontend (Login.vue): Loads login page
Frontend (Login.vue)->>Backend API (get_user_pass_login_disabled): GET disable_user_pass_login
Backend API->>System Settings: Retrieve disable_user_pass_login
System Settings-->>Backend API: Return flag value
Backend API-->>Frontend (Login.vue): Return flag value
alt User-password login enabled
Frontend (Login.vue)-->>User: Show user-password login form and/or OAuth options
else User-password login disabled and no OAuth
Frontend (Login.vue)-->>User: Show "No login methods available" message
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
hrms/api/system_settings.py (1)
4-5
: Consider adding explicit boolean conversion for clarity.While the current implementation works, consider ensuring the return value is explicitly boolean for better API consistency:
def get_user_pass_login_disabled(): - return frappe.get_system_settings("disable_user_pass_login") + return bool(frappe.get_system_settings("disable_user_pass_login"))frontend/src/views/Login.vue (1)
170-170
: Remove unnecessary method specification.The
method: 'GET'
is redundant since GET is the default method forcreateResource
.const user_pass_login_disabled = createResource({ url: "hrms.api.system_settings.get_user_pass_login_disabled", - method: 'GET', initialData: [1], auto: true, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/views/Login.vue
(4 hunks)hrms/api/system_settings.py
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (4)
hrms/api/system_settings.py (1)
1-6
: Clean and focused API endpoint implementation.The implementation is straightforward and serves its purpose well. The
allow_guest=True
decorator is appropriate since the login page needs to check this setting before authentication.frontend/src/views/Login.vue (3)
13-13
: Good conditional rendering implementation.The conditional rendering of the login form based on the system setting is well implemented and improves user experience by preventing unnecessary form submission attempts.
39-39
: Proper conditional rendering of separator text.Good attention to detail in also conditionally rendering the "or" separator to maintain clean UI when the login form is hidden.
53-53
: Excellent fallback message for better UX.The informative message when no login methods are available provides clear guidance to users and significantly improves the user experience.
Fixes #3422
This PR improves the Login-UX by conditionally displaying the Username/Password Login only if it is not disabled in the System Settings.
Currently the login form is always shown, even when username/password login is disabled. The user only sees an error after attempting to log in.
A new API endpoint was added to fetch the
disable_user_pass_login
setting.Login.vue
uses this setting to determine whether to render the username/password login form.Summary by CodeRabbit
New Features
Bug Fixes