feat(auth): display server-side auth config status on signin/signup - #3642
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR implements server-side authentication configuration status checking. A new Changes
Possibly related issues
Suggested labels
Poem
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3642 +/- ##
==========================================
+ Coverage 98.52% 98.55% +0.03%
==========================================
Files 19 19
Lines 473 485 +12
Branches 119 121 +2
==========================================
+ Hits 466 478 +12
Misses 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds frontend awareness of server-side auth enablement flags so the signin/signup pages can hide their forms and show a warning when the backend disables those flows (per #3641).
Changes:
- Added
serverConfigstate and afetchServerConfig()action to the auth Pinia store. - Updated signin/signup views to fetch server auth config on creation and conditionally render an alert vs. the form.
- Updated/extended unit tests to account for the new store state/action.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/modules/auth/stores/auth.store.js |
Adds serverConfig state and fetchServerConfig() action that calls the new public API endpoint. |
src/modules/auth/views/auth.signin.view.vue |
Fetches server config and conditionally shows a warning + hides signin form when disabled. |
src/modules/auth/views/auth.signup.view.vue |
Fetches server config and conditionally shows a warning + hides signup form when disabled. |
src/modules/auth/tests/auth.store.spec.js |
Adds unit coverage for default serverConfig state and fetchServerConfig() behavior. |
src/modules/auth/tests/auth.signin.view.spec.js |
Updates auth store mock to include serverConfig + fetchServerConfig(). |
src/modules/auth/tests/auth.signup.view.spec.js |
Updates auth store mock to include serverConfig + fetchServerConfig(). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/modules/auth/views/auth.signup.view.vue (1)
70-90:⚠️ Potential issue | 🟡 MinorAdd the required JSDoc blocks for the modified hooks.
data()and the new asynccreated()hook are both modified functions, but neither has the repo-required JSDoc header.As per coding guidelines
src/**/*.{js,ts,vue}: Every new or modified function must have a JSDoc header with one-line description,@paramfor each argument, and@returnsfor any non-void return value (always include@returnsfor async functions).Also applies to: 106-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/auth/views/auth.signup.view.vue` around lines 70 - 90, Add JSDoc headers to the modified data() and created() functions in this Vue component: for each function include a one-line description, an `@param` for each argument (even if none, document as `@param` {void} or `@param` _unused), and an `@returns` describing the return type (for data() the returned state object, and for async created() include `@returns` {Promise<void>} or appropriate promise type); ensure headers follow the repo convention and appear immediately above the data() and created() function definitions.src/modules/auth/views/auth.signin.view.vue (1)
74-90:⚠️ Potential issue | 🟡 MinorAdd the required JSDoc blocks for the modified hooks.
data()and the new asynccreated()hook are modified functions without JSDoc headers.As per coding guidelines
src/**/*.{js,ts,vue}: Every new or modified function must have a JSDoc header with one-line description,@paramfor each argument, and@returnsfor any non-void return value (always include@returnsfor async functions).Also applies to: 106-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/auth/views/auth.signin.view.vue` around lines 74 - 90, Add JSDoc headers for the modified Vue component hooks data() and the async created() so they follow project guidelines: for each function (data and created) add a one-line description, include `@param` tags for any arguments (even if none, document as none or omit params per project convention) and include `@returns` describing the return value (for data() return object shape, and for async created() include `@returns` Promise<void> or the actual Promise result). Ensure the JSDoc appears immediately above the function definitions (data() and created()) and covers any changed behavior or returned values referenced in the function body.
🧹 Nitpick comments (2)
src/modules/auth/tests/auth.signin.view.spec.js (1)
7-9: Please cover the disabled-signin path here.This suite now knows about
fetchServerConfig(), but it still doesn't verify the new behavior introduced by the PR. Add a mount-time case where the fetched config setssign.in: false, then assert the warning is shown and the signin form is not rendered.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/auth/tests/auth.signin.view.spec.js` around lines 7 - 9, Add a test case that mocks useAuthStore to have fetchServerConfig return a serverConfig object with sign: { in: false } at mount time, then mount the SignIn view/component and assert that the "signin disabled" warning is rendered and the signin form elements are not present; update the vi.mock call (useAuthStore, fetchServerConfig, signinMock) in auth.signin.view.spec.js to include this scenario, mount the component (the same helper used elsewhere in the file), await any pending promises, and use assertions to check the warning is visible and the signin form is not rendered.src/modules/auth/tests/auth.signup.view.spec.js (1)
7-9: Please cover the disabled-signup path here.This mock update is necessary, but the suite still never asserts the new behavior from this PR. Add a case where
fetchServerConfig()resolves a config withsign.up: false, then verify the warning is rendered and the<v-form>is hidden.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/auth/tests/auth.signup.view.spec.js` around lines 7 - 9, Add a new test in auth.signup.view.spec.js that covers the disabled-signup path by having the mocked useAuthStore's fetchServerConfig resolve to a config object with sign.up: false (e.g., make fetchServerConfig.mockResolvedValue({ sign: { up: false } }) for that test), then mount/render the signup view, await any async resolution, and assert that the warning message is rendered and that the <v-form> is not present; reference the existing useAuthStore / signupMock / fetchServerConfig mocks so you only override fetchServerConfig for this test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/auth/views/auth.signin.view.vue`:
- Around line 10-11: The signin form is rendered before the async config loads
because serverConfig starts as null; add an explicit ready flag (e.g.,
serverConfigReady or configLoaded) and use it to gate both the v-alert and the
v-form (render only when serverConfigReady is true and then check
serverConfig.sign.in), update the template conditions around v-alert and v-form
to include this flag, set serverConfigReady = false in the data() function and
flip it to true at the end of the async created() hook after loading
serverConfig, and add JSDoc headers to the modified data() and created()
functions as per coding guidelines; look for the symbols serverConfig, data(),
created(), and the v-form/v-alert template lines to apply the change.
In `@src/modules/auth/views/auth.signup.view.vue`:
- Around line 10-11: The signup form flashes because it renders before the async
config request completes; change the template condition to only show/hide the
form after the config has settled (introduce a boolean like serverConfigLoaded
or use serverConfig !== null) and update the v-if on the <v-form> (currently
using serverConfig and serverConfig.sign.up) to require the loaded flag and
serverConfig.sign.up, and show the warning only when loaded &&
!serverConfig.sign.up; also add JSDoc headers for the Vue Options API methods:
add proper JSDoc comments above the data() function and the async created() hook
(referenced as data() and created()) describing return types and purpose.
---
Outside diff comments:
In `@src/modules/auth/views/auth.signin.view.vue`:
- Around line 74-90: Add JSDoc headers for the modified Vue component hooks
data() and the async created() so they follow project guidelines: for each
function (data and created) add a one-line description, include `@param` tags for
any arguments (even if none, document as none or omit params per project
convention) and include `@returns` describing the return value (for data() return
object shape, and for async created() include `@returns` Promise<void> or the
actual Promise result). Ensure the JSDoc appears immediately above the function
definitions (data() and created()) and covers any changed behavior or returned
values referenced in the function body.
In `@src/modules/auth/views/auth.signup.view.vue`:
- Around line 70-90: Add JSDoc headers to the modified data() and created()
functions in this Vue component: for each function include a one-line
description, an `@param` for each argument (even if none, document as `@param`
{void} or `@param` _unused), and an `@returns` describing the return type (for
data() the returned state object, and for async created() include `@returns`
{Promise<void>} or appropriate promise type); ensure headers follow the repo
convention and appear immediately above the data() and created() function
definitions.
---
Nitpick comments:
In `@src/modules/auth/tests/auth.signin.view.spec.js`:
- Around line 7-9: Add a test case that mocks useAuthStore to have
fetchServerConfig return a serverConfig object with sign: { in: false } at mount
time, then mount the SignIn view/component and assert that the "signin disabled"
warning is rendered and the signin form elements are not present; update the
vi.mock call (useAuthStore, fetchServerConfig, signinMock) in
auth.signin.view.spec.js to include this scenario, mount the component (the same
helper used elsewhere in the file), await any pending promises, and use
assertions to check the warning is visible and the signin form is not rendered.
In `@src/modules/auth/tests/auth.signup.view.spec.js`:
- Around line 7-9: Add a new test in auth.signup.view.spec.js that covers the
disabled-signup path by having the mocked useAuthStore's fetchServerConfig
resolve to a config object with sign.up: false (e.g., make
fetchServerConfig.mockResolvedValue({ sign: { up: false } }) for that test),
then mount/render the signup view, await any async resolution, and assert that
the warning message is rendered and that the <v-form> is not present; reference
the existing useAuthStore / signupMock / fetchServerConfig mocks so you only
override fetchServerConfig for this test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d537636-1f20-481b-807b-9aa94887747c
📒 Files selected for processing (6)
src/modules/auth/stores/auth.store.jssrc/modules/auth/tests/auth.signin.view.spec.jssrc/modules/auth/tests/auth.signup.view.spec.jssrc/modules/auth/tests/auth.store.spec.jssrc/modules/auth/views/auth.signin.view.vuesrc/modules/auth/views/auth.signup.view.vue
Summary
serverConfigstate andfetchServerConfig()action to the auth store. Signin and signup views now fetch the server auth config on mount and display a warning alert when sign.in or sign.up is disabled, hiding the form.Scope
auth(store, signin view, signup view, tests)nonelowValidation
npm run lintnpm run test:unitnpm run buildGuardrails check
.env*,secrets/**, keys, tokens)Notes for reviewers
/api/auth/configendpoint is a public GET route exposing only boolean flags (sign.in, sign.up). No sensitive data is fetched.Summary by CodeRabbit
New Features
Tests