fix(setup): derive signin URL from api_url for self-hosted instances#131
Open
manumishra12 wants to merge 1 commit into
Open
fix(setup): derive signin URL from api_url for self-hosted instances#131manumishra12 wants to merge 1 commit into
manumishra12 wants to merge 1 commit into
Conversation
The signin/re-authenticate flow always redirected to the cloud
hexmos.com domain, even when git-lrc was configured against a
self-hosted LiveReview instance. BuildSigninURL ignored the api_url
and always parsed the hardcoded HexmosSigninBase constant, so
self-hosted users saw 'not authenticated' and re-auth bounced them
to hexmos.com instead of their own server.
Thread api_url into BuildSigninURL and resolve the signin base via
ResolveSigninBase:
1. LRC_SIGNIN_URL override, if set;
2. cloud default (hexmos.com/signin) when api_url is empty or the
cloud URL -- cloud behavior is unchanged;
3. otherwise <api_url>/signin, so a self-hosted instance signs in
against itself.
Also relax the signin base to allow http (local self-hosted
instances), still requiring a valid scheme and host. Covers both
initial setup and the UI re-authenticate button, which share this
code path.
Fixes HexmosTech#28
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #28 — when
git-lrcis configured against a self-hosted LiveReview instance, the signin / "Re-authenticate" flow always redirected to the cloudhexmos.comdomain instead of the self-hosted server. Self-hosted users sawnot authenticated, and clicking Re-authenticate bounced them tohexmos.comeven though review-on-commit requests were correctly going to their local instance.Root cause
runHexmosLoginFlowalready had the configuredapiURLin scope (self-hosted-aware), butBuildSigninURL(callbackURL)ignored it and always parsed the hardcoded cloud constantHexmosSigninBase = "https://hexmos.com/signin". Both initial setup and the UI re-authenticate button share this code path, so both were affected.Fix
Thread
apiURLintoBuildSigninURLand resolve the signin base via a newResolveSigninBase(apiURL)with this precedence:LRC_SIGNIN_URLenv override, if set;hexmos.com/signin) whenapi_urlis empty or the cloud URL — cloud behavior is unchanged;<api_url>/signin, so a self-hosted instance signs in against itself.Also relaxed the signin base validation to allow
http(local self-hosted instances), still requiring a valid scheme and a non-empty host. The<api_url>/signinderivation mirrors the existingnetwork/endpoints.goconvention (strings.TrimSuffix(apiURL, "/") + path).The
<api_url>/signinpath is a reasonable assumption — I couldn't find a documented convention in the repo for where a self-hosted LiveReview serves its signin page. If that path is wrong, theLRC_SIGNIN_URLoverride means self-hosters aren't blocked, but please confirm the correct path so the derived default can be corrected if needed.Testing
go build ./...,go vet,gofmt -lall cleansetupandinternal/appuipackages pass, including newBuildSigninURLtests covering: cloud default, empty api_url, self-hosted derivation, trailing-slash api_url, and theLRC_SIGNIN_URLoverride.