Skip to content

fix(config-service): expose inviteOnly on /config/pre-login so INACTIVE users see the registration-request form#5572

Open
aicam wants to merge 5 commits into
apache:mainfrom
aicam:fix/user-system-config-permitall
Open

fix(config-service): expose inviteOnly on /config/pre-login so INACTIVE users see the registration-request form#5572
aicam wants to merge 5 commits into
apache:mainfrom
aicam:fix/user-system-config-permitall

Conversation

@aicam

@aicam aicam commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

#5305 moved GET /config/user-system from @PermitAll to
@RolesAllowed("REGULAR", "ADMIN"). A freshly-registered user is INACTIVE
until admin approval, so they cannot reach @RolesAllowed endpoints — the
request returns 403/401, inviteOnly is left undefined on the frontend, the
registration-request form never appears, and no admin notification email is
sent. In invite-only deployments, new sign-ups are silently dropped.

Per review feedback (@Yicong-Huang), instead of re-opening the whole
/config/user-system endpoint with @PermitAll, this PR exposes only the
inviteOnly boolean on the already-public /config/pre-login and keeps
/config/user-system @RolesAllowed. The frontend already loads
/config/pre-login anonymously during APP_INITIALIZER, so inviteOnly is now
available before activation without widening the authenticated surface.

Any related issues, documentation, discussions?

Resolves #5587

How was this PR tested?

  • Updated ConfigResourceAuthSpec: /config/pre-login exposes exactly
    {localLogin, googleLogin, defaultLocalUser, attributionEnabled, inviteOnly}
    anonymously; /config/user-system returns 401 + Bearer challenge without a
    token and 200 with a valid Bearer token. sbt ConfigService/test → 9 passed.
  • Verified live on an invite-only deployment: a fresh INACTIVE registration
    reads inviteOnly: true from /config/pre-login, the registration-request
    form appears, and the admin notification email is sent, while
    /config/user-system still returns 403/401 to anonymous callers.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 4.8)

@github-actions github-actions Bot added fix platform Non-amber Scala service paths labels Jun 8, 2026
…sers can read inviteOnly

A freshly-registered user is INACTIVE until an admin approves them and
therefore cannot reach the @RolesAllowed("REGULAR", "ADMIN") config
endpoints. The frontend reads the `inviteOnly` flag at exactly that point
to decide whether to show the registration-request form (and notify
admins). Since apache#5305 moved /config/user-system behind a role check, the
flag became unreachable for the very users it targets, so the form never
appeared and no admin notification was sent.

Restore @permitAll on /config/user-system. It only exposes the boolean
inviteOnly flag, which is non-sensitive and already needed pre-activation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aicam aicam force-pushed the fix/user-system-config-permitall branch from 8d3763f to 8e6a1b4 Compare June 8, 2026 22:50
@aicam aicam requested a review from bobbai00 June 8, 2026 22:50
@codecov-commenter

codecov-commenter commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.42%. Comparing base (cd60535) to head (7c56d25).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #5572   +/-   ##
=========================================
  Coverage     52.42%   52.42%           
- Complexity     2481     2482    +1     
=========================================
  Files          1070     1070           
  Lines         41359    41363    +4     
  Branches       4441     4441           
=========================================
+ Hits          21682    21686    +4     
  Misses        18406    18406           
  Partials       1271     1271           
Flag Coverage Δ *Carryforward flag
access-control-service 64.61% <ø> (ø)
agent-service 33.76% <ø> (ø) Carriedforward from cd60535
amber 53.28% <ø> (ø) Carriedforward from cd60535
computing-unit-managing-service 1.65% <ø> (ø)
config-service 58.57% <100.00%> (+2.51%) ⬆️
file-service 38.21% <ø> (ø)
frontend 47.03% <ø> (ø) Carriedforward from cd60535
pyamber 90.72% <ø> (ø) Carriedforward from cd60535
python 90.75% <ø> (ø) Carriedforward from cd60535
workflow-compiling-service 58.69% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…nfig/user-system

The spec pinned /config/user-system as @RolesAllowed (401 without a token).
Now that the endpoint is @permitAll, assert it answers anonymous callers
with 200 and exposes exactly the inviteOnly flag, while still serving
authenticated callers. Mirrors the existing /config/pre-login guard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bobbai00

bobbai00 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

The PR description is not following the PR template. Please fix the PR description.

Also, please create an issue and resolve that issue in this PR.

Since it is related to user experience, please include screenshots of BEFORE & AFTER.

@bobbai00 bobbai00 requested a review from xuang7 June 9, 2026 19:11
bobbai00
bobbai00 previously approved these changes Jun 9, 2026
@bobbai00

bobbai00 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@xuang7 I think this needs to be included in the release v1.2 . Can you review it and decide ?

@bobbai00 bobbai00 self-requested a review June 9, 2026 19:13
@bobbai00 bobbai00 dismissed their stale review June 9, 2026 19:14

Needs Xuan's input on release

@xuang7

xuang7 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

test @aicam

@xuang7 xuang7 added the release/v1.2 back porting to release/v1.2 label Jun 9, 2026

@Yicong-Huang Yicong-Huang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move needed endpoint (e.g., invite-only config) to pre-login which is exposed to all. don't expose all endpoints just because one of the flag needs to be exposed.

@aicam aicam changed the title fix(config-service): make /config/user-system anonymous so INACTIVE users can read inviteOnly fix(config-service): expose inviteOnly on /config/pre-login so INACTIVE users see the registration-request form Jun 10, 2026
@aicam aicam force-pushed the fix/user-system-config-permitall branch from c0cf8b7 to 404733b Compare June 10, 2026 15:43
A freshly-registered user is INACTIVE and cannot reach the @RolesAllowed
config endpoints, yet the frontend reads inviteOnly right after registration to
decide whether to show the registration-request form (and notify admins).
apache#5305 moved inviteOnly to /config/user-system (@RolesAllowed), making it
unreachable for exactly those users. Expose the flag on the already-public
@permitAll /config/pre-login instead, without widening any authenticated
endpoint.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aicam aicam force-pushed the fix/user-system-config-permitall branch from 404733b to 0f879fb Compare June 10, 2026 15:48
@aicam

aicam commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

please move needed endpoint (e.g., invite-only config) to pre-login which is exposed to all. don't expose all endpoints just because one of the flag needs to be exposed.

Thanks, fixed, could you please check again

@aicam aicam requested a review from Yicong-Huang June 10, 2026 16:45

@Yicong-Huang Yicong-Huang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix platform Non-amber Scala service paths release/v1.2 back porting to release/v1.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invite-only registration-request form never appears: /config/user-system returns 403 for INACTIVE users

5 participants