fix(oidc): portal-origin logins provision customer-type users - #19
Conversation
The support portal's DOS ID button round-trips a /support/* return path, so the signed state already identifies the entry surface. Use it to decide provisioning at creation time: - portal-origin first logins create UserTypeUser customers with no staff role, no org/team sync, no agent profile; the break-glass allowlist never elevates on the portal surface - staff-surface logins keep the Wave 2 behaviour (employee type plus claim/allowlist-derived roles) - user type is fixed at creation and never flipped by the other surface (fail-closed; the dashboard middleware already rejects non-employees) Before this, a customer signing in through the portal became an employee user (and, before the redirect-only change, received the admin role - a defect inherited from upstream agent-desk, which still grants admin to every first-time OIDC user).
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces logic to distinguish OIDC logins originating from the customer support portal from those starting at the staff dashboard, ensuring portal-origin logins provision customer-type users without staff roles or profiles. The review feedback highlights a critical security vulnerability where query parameters or URL fragments in the redirect path would bypass the support portal detection, incorrectly provisioning customers as employees. Additionally, the feedback suggests a defense-in-depth check to restrict staff provisioning to employee-type users and recommends adding corresponding test cases for URL parameter and fragment handling.
| func isSupportPortalNext(next string) bool { | ||
| next = strings.TrimSpace(next) | ||
| return next == "/support" || strings.HasPrefix(next, "/support/") | ||
| } |
There was a problem hiding this comment.
The current implementation of isSupportPortalNext does not handle query parameters or URL fragments (e.g., /support?ticket=123 or /support/tickets?resolved=true#anchor). If a customer logs in via the support portal with any query parameters or fragments in the redirect path, isSupportPortalNext will return false. This causes them to be incorrectly provisioned as an employee (UserTypeEmployee) and receive default staff roles, which is a critical security vulnerability (privilege escalation).
We should strip query parameters and fragments before performing the prefix and equality checks.
func isSupportPortalNext(next string) bool {
next = strings.TrimSpace(next)
if idx := strings.IndexAny(next, "?#"); idx != -1 {
next = next[:idx]
}
return next == "/support" || strings.HasPrefix(next, "/support/")
}| if !portalOrigin { | ||
| s.ensureDefaultOIDCRole(ctx.Tx, user) | ||
| s.ensureBreakGlassAdminRole(ctx.Tx, authCfg, user, profile) | ||
| s.syncOIDCUserOrganizations(ctx.Tx, user, profile) | ||
| s.syncOIDCUserTeams(ctx.Tx, user, profile) | ||
| _, _ = AgentProfileService.EnsureAgentProfileForUser(ctx.Tx, user) | ||
| } |
There was a problem hiding this comment.
As a defense-in-depth measure, we should ensure that staff provisioning (roles, organization/team sync, and agent profile creation) is only executed if the user is actually an employee (UserTypeEmployee). Currently, if an existing customer (UserTypeUser) attempts to log in via the staff surface (portalOrigin == false), they would still have staff roles, organizations, and agent profiles provisioned for them in the database, even though they are rejected by the dashboard middleware.
| if !portalOrigin { | |
| s.ensureDefaultOIDCRole(ctx.Tx, user) | |
| s.ensureBreakGlassAdminRole(ctx.Tx, authCfg, user, profile) | |
| s.syncOIDCUserOrganizations(ctx.Tx, user, profile) | |
| s.syncOIDCUserTeams(ctx.Tx, user, profile) | |
| _, _ = AgentProfileService.EnsureAgentProfileForUser(ctx.Tx, user) | |
| } | |
| if !portalOrigin && user.UserType == enums.UserTypeEmployee { | |
| s.ensureDefaultOIDCRole(ctx.Tx, user) | |
| s.ensureBreakGlassAdminRole(ctx.Tx, authCfg, user, profile) | |
| s.syncOIDCUserOrganizations(ctx.Tx, user, profile) | |
| s.syncOIDCUserTeams(ctx.Tx, user, profile) | |
| _, _ = AgentProfileService.EnsureAgentProfileForUser(ctx.Tx, user) | |
| } |
| {next: "/support/community", expected: true}, | ||
| {next: "/support", expected: true}, | ||
| {next: " /support/tickets ", expected: true}, | ||
| {next: "/dashboard", expected: false}, | ||
| {next: "", expected: false}, | ||
| {next: "/supportevil", expected: false}, |
There was a problem hiding this comment.
Add test cases to verify that isSupportPortalNext correctly handles query parameters and URL fragments.
{next: "/support/community", expected: true},
{next: "/support", expected: true},
{next: " /support/tickets ", expected: true},
{next: "/support?ticket=123", expected: true},
{next: "/support/tickets?resolved=true#anchor", expected: true},
{next: "/dashboard", expected: false},
{next: "", expected: false},
{next: "/supportevil", expected: false},…ortal note to OIDC
1 participant
Follow-up to #18 completing the everyone-signs-in-with-DOS-ID model for the customer support portal.
The portal's DOS ID button round-trips a /support/* return path inside the signed OIDC state, so the entry surface is already identifiable. Portal-origin first logins now provision customer-type users (UserTypeUser) with no staff role, no organization/team provisioning, no agent profile, and no break-glass elevation. Staff-surface logins keep the #18 behaviour (employee type plus claim/allowlist-derived roles). User type is fixed at creation and never flipped by the other surface (fail-closed: the dashboard middleware already rejects non-employee users).
Before this, a customer signing in through the portal became an employee user - and before #18, received the admin role. That defect is inherited from upstream huabeitech/agent-desk, which still grants admin to every first-time OIDC user (verified against upstream/main).
Verification: new tests cover portal-origin provisioning (customer type, zero roles/orgs/agent profiles), allowlist-ignoring on the portal surface, and the /support prefix matcher; existing staff-surface OIDC tests updated and passing; go build + go vet clean.