feat: add RedirectUriPrefix support to OpenIdConnectConfiguration for sub-path deployments#809
Conversation
Greptile SummaryThis PR adds a
Confidence Score: 4/5Safe to merge after fixing the corrupted ellipsis character in the error-logging path; the OIDC feature itself is correctly implemented. The redirect URI construction change is correct and backward-compatible. The one concrete defect is an unintended character corruption on line 136 of OpenIdConnectAuthorizationService.cs — the ellipsis was replaced with the Unicode Replacement Character when the BOM was stripped, meaning every truncated OIDC error log entry will end with a garbled glyph. src/modules/Elsa.Studio.Login/Services/OpenIdConnectAuthorizationService.cs — specifically line 136 where the ellipsis character was corrupted.
|
| Filename | Overview |
|---|---|
| src/modules/Elsa.Studio.Login/Services/OpenIdConnectAuthorizationService.cs | Adds RedirectUriPrefix support to redirect URI construction; inadvertently corrupts the ellipsis character (U+2026 to U+FFFD) in ReadErrorSummaryAsync line 136 due to BOM removal. |
| src/modules/Elsa.Studio.Login/Extensions/StringExtensions.cs | New helper file adding EnsureStartsWith and EnsureEndsWith string extensions; EnsureEndsWith is currently unused dead code. |
| src/modules/Elsa.Studio.Login/Models/OpenIdConnectConfiguration.cs | Adds nullable RedirectUriPrefix property with clear XML doc comment; no issues. |
Sequence Diagram
sequenceDiagram
participant Browser
participant ElsaStudio as Elsa Studio (Blazor)
participant Config as OpenIdConnectConfiguration
participant IdP as Identity Provider
Browser->>ElsaStudio: Navigate to protected route
ElsaStudio->>Config: Read RedirectUriPrefix (e.g. "/workflow")
ElsaStudio->>ElsaStudio: "Build redirectUri = origin + EnsureStartsWith + /signin-oidc"
Note over ElsaStudio: e.g. https://myapp.com/workflow/signin-oidc
ElsaStudio->>IdP: GET /authorize with redirect_uri
IdP-->>Browser: 302 to redirect_uri with auth code
Browser->>ElsaStudio: "GET /workflow/signin-oidc?code=AUTH_CODE"
ElsaStudio->>Config: Read RedirectUriPrefix again
ElsaStudio->>ElsaStudio: Rebuild same redirectUri for token exchange
ElsaStudio->>IdP: POST /token with code and redirect_uri
IdP-->>ElsaStudio: access_token, refresh_token, id_token
ElsaStudio-->>Browser: NavigateTo returnUrl
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/modules/Elsa.Studio.Login/Extensions/StringExtensions.cs:19-28
`EnsureEndsWith` is defined but never called anywhere in this PR or the existing codebase. Shipping unused internal code creates unnecessary maintenance surface. If it's genuinely needed in a follow-up, it can be added then; otherwise it should be removed.
### Issue 2 of 2
src/modules/Elsa.Studio.Login/Services/OpenIdConnectAuthorizationService.cs:136
**Corrupted ellipsis character**
The original file (which had a UTF-8 BOM) contained the HORIZONTAL ELLIPSIS `…` (U+2026) on this line. When the BOM was stripped in this PR, the multi-byte sequence for that character was corrupted to U+FFFD (the Unicode Replacement Character). At runtime, truncated error summaries logged by `ReadErrorSummaryAsync` will end with the replacement character `<?>` instead of `…`, producing garbled log output. Replace the character literal on this line with the correct `…` (U+2026), or use the escape `\u2026`.
Reviews (2): Last reviewed commit: "Merge branch 'main' into feat-adding-Red..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@sfmskywalker |
|
@greptile please review |
| summary = summary[..MaxLoggedErrorLength]; | ||
|
|
||
| return truncated ? $"{summary}…" : summary; | ||
| return truncated ? $"{summary}�" : summary; |
There was a problem hiding this comment.
The original file (which had a UTF-8 BOM) contained the HORIZONTAL ELLIPSIS … (U+2026) on this line. When the BOM was stripped in this PR, the multi-byte sequence for that character was corrupted to U+FFFD (the Unicode Replacement Character). At runtime, truncated error summaries logged by ReadErrorSummaryAsync will end with the replacement character <?> instead of …, producing garbled log output. Replace the character literal on this line with the correct … (U+2026), or use the escape \u2026.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/modules/Elsa.Studio.Login/Services/OpenIdConnectAuthorizationService.cs
Line: 136
Comment:
**Corrupted ellipsis character**
The original file (which had a UTF-8 BOM) contained the HORIZONTAL ELLIPSIS `…` (U+2026) on this line. When the BOM was stripped in this PR, the multi-byte sequence for that character was corrupted to U+FFFD (the Unicode Replacement Character). At runtime, truncated error summaries logged by `ReadErrorSummaryAsync` will end with the replacement character `<?>` instead of `…`, producing garbled log output. Replace the character literal on this line with the correct `…` (U+2026), or use the escape `\u2026`.
How can I resolve this? If you propose a fix, please make it concise.2 participants
Purpose
Allow applications using the ElsaLogin OIDC flow to configure a path prefix for the redirect_uri,
enabling correct authentication in sub-path deployments where the IdP enforces a specific redirect_uri format.
Scope
Select one primary concern:
This produces
https://myapp.com/workflow/signin-oidcinstead ofhttps://myapp.com/signin-oidc.When not set, behaviour is unchanged — defaults to
{origin}/signin-oidc.Verification
Steps:
/workflow).Authentication:ElsaLogin:RedirectUriPrefixto/workflowinappsettings.json.https://myapp.com/workflow/signin-oidcas an allowed redirect URI in your IdP(e.g., Azure AD, Keycloak).
Expected outcome: The authorization request is sent with
redirect_uri=https://myapp.com/workflow/signin-oidc, the IdP accepts it, and the user issuccessfully authenticated.
Screenshots / Recordings (if applicable)
N/A — No UI changes. This is a configuration and service-layer change only.
Checklist