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
|
@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.|
PR author is not in the allowed authors list. |
|
appreciate the wonderful package, any update on this we really need this feature |
|
any updates on this? |
|
@IbrahimNadaa please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement ( “Agreement” ) is agreed to by the party signing below ( “You” ), 1. Definitions. “Code” means the computer software code, whether in human-readable or machine-executable form, “Project” means any of the projects owned or managed by .NET Foundation and offered under a license “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any “Submission” means the Code and any other copyrightable material Submitted by You, including any 2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any 3. Originality of Work. You represent that each of Your Submissions is entirely Your 4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else 5. Licenses. a. Copyright License. You grant .NET Foundation, and those who receive the Submission directly b. Patent License. You grant .NET Foundation, and those who receive the Submission directly or c. Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement. 6. Representations and Warranties. You represent that You are legally entitled to grant the above 7. Notice to .NET Foundation. You agree to notify .NET Foundation in writing of any facts or 8. Information about Submissions. You agree that contributions to Projects and information about 9. Governing Law/Jurisdiction. This Agreement is governed by the laws of the State of Washington, and 10. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and .NET Foundation dedicates this Contribution License Agreement to the public domain according to the Creative Commons CC0 1. |
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