fix: handle studioctl auth username casing#18816
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✨ 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 |
|
how does the username (in the toke?) get the wrong casing? Before your change |
|
Before this change the username used by For Studio OIDC that name is set in SELECT u.lower_name
FROM external_login_user elu
JOIN "user" u ON elu.user_id = u.id
WHERE elu.external_id = @sub
LIMIT 1So for a Gitea user where This PR keeps using the computed/authenticated username for pending-request ownership, but asks Gitea |
|
@martinothamar-agent |
|
Good question. It probably should not be case-sensitive for identity in this context. I checked how Gitea handles this: route/API params such as The case-sensitive part here is on the Designer settings side: the settings route owner is compared to the current user login as a string. With Studio OIDC, the auth claim currently comes from Gitea So I agree the more Gitea-like invariant is: compare owner/usernames case-insensitively for identity, and use canonical casing only when generating/normalizing URLs. This PR fixes the studioctl entrypoint by generating the canonical URL, but the broader cleanup would be to make the settings owner checks case-insensitive too. |
|
Proposed by @mirkoSekulic internally:
lets do that |
2610aa2 to
ca1b35d
Compare
|
Updated the PR accordingly. Changes now follow the proposed direction:
I also manually tested the risky path locally with an existing lower-case Designer mapping ( |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #18816 +/- ##
========================================
Coverage 95.84% 95.84%
========================================
Files 3018 3023 +5
Lines 39575 39697 +122
Branches 4849 4885 +36
========================================
+ Hits 37931 38049 +118
- Misses 1230 1233 +3
- Partials 414 415 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3a170ce to
9190db8
Compare
|
Addressed the Codecov coverage complaint in the latest push. I added explicit null-user coverage for both Local verification now reports 100% branch coverage for both touched layout files with:
Also repeated the manual Designer flow after the rebase; canonical URL, lowercase settings URL, userinfo, and DB mapping migration all passed. |
|
Already stored user data should be immutable. |
|
Addressed Mirko's feedback in |
8ec94e7 to
ed21b33
Compare
Jondyr
left a comment
There was a problem hiding this comment.
Tested in dev, found no issues. nice! 👍
What
user.nameinstead ofuser.lower_name, soClaimTypes.Nameuses canonical casing such asJondyr.UserAccountsusername when it only differs from the linked Gitea username by casing.Why
Gitea preserves canonical username casing in
name/ APIlogin, while usinglower_namefor case-insensitive lookup and uniqueness. Designer was leakinglower_nameinto the Studio OIDC auth claim, and settings route checks compared owner values case-sensitively. That could make studioctl redirect to/settings/jondyr/studioctl-authfor a canonical userJondyr.Testing
dotnet test src/Designer/backend/tests/Designer.Tests/Designer.Tests.csproj --filter StudioctlAuthServiceTests --logger "console;verbosity=minimal"corepack yarn jest settings/layouts/UserPageLayout/UserPageLayout.test.tsx settings/layouts/OrgPageLayout/OrgPageLayout.test.tsx settings/components/OwnerIndexRedirect/OwnerIndexRedirect.test.tsx settings/features/user/pages/StudioctlAuth/StudioctlAuth.test.tsx --runInBand --config=jest.config.js --coverage --collectCoverageFrom='settings/layouts/UserPageLayout/UserPageLayout.tsx' --collectCoverageFrom='settings/layouts/OrgPageLayout/OrgPageLayout.tsx'http://studio.localhost.Jondyrwithlower_name=jondyr.user_accounts.username = jondyr./Login-> fake Ansattporten -> OIDC callback ->designer/api/v1/studioctl/auth/authorize./designer/api/v1/studio-oidc/userinforeturnedusername: "Jondyr".Jondyr./settings/Jondyr/studioctl-auth?requestId=...with HTTP 200./settings/jondyr/studioctl-auth?requestId=...loaded the same auth request instead of 404.cc @martinothamar