fix: --app flag not applied to subcommands#46
Open
rmacias12 wants to merge 1 commit intoxdevplatform:mainfrom
Open
fix: --app flag not applied to subcommands#46rmacias12 wants to merge 1 commit intoxdevplatform:mainfrom
rmacias12 wants to merge 1 commit intoxdevplatform:mainfrom
Conversation
The --app flag was silently ignored across the codebase. While WithAppName correctly stored the app name, every subsequent token read, write, and clear still resolved to the default app. - fix token reads in auth.go (GetOAuth1Header, GetOAuth2Header, RefreshOAuth2Token, GetBearerTokenHeader) to use ForApp variants - fix OAuth2 token saves in OAuth2Flow and RefreshOAuth2Token to write back to the named app instead of the default - fix WithAppName to replace credentials even when env vars were already set - fix auto-detection probes in api/client.go (getAuthHeader) to check the named app for available tokens - fix cli/auth.go save and clear commands (bearer, oauth1, clear) to operate on the named app - fix cli/webhook.go CRC validation to read OAuth1 secret from the named app - add AppName() getter on Auth to expose appName to the api package - add tests covering token isolation per app, credential override, refresh token save target, and default-app regression guards
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.
Fix:
--appflag not applied to subcommandsFixes /issues/38
Similar to PR #39 with few extra areas covered.
Summary
The global
--appflag was silently ignored when used with any subcommand(e.g.
xurl whoami --app my-app). The command would always authenticateusing the default app instead of the specified one. A comprehensive audit
found the same class of bug in six files across the codebase.
Root Cause
The
--appflag correctly stored the app name ina.appNameviaWithAppName, but every place that subsequently read tokens, saved tokens,or cleared tokens called the non-
ForAppstore variants, which always pass""toResolveAppand therefore always resolve to the default app.Bugs Fixed
Bug 1 — Token reads in
auth/auth.goignored--appEvery method that retrieves tokens called the non-
ForAppstore variants:GetOAuth1HeaderTokenStore.GetOAuth1Tokens()a.appNameGetOAuth2HeaderTokenStore.GetOAuth2Token(username)/GetFirstOAuth2Token()a.appNameRefreshOAuth2TokenTokenStore.GetOAuth2Token(username)/GetFirstOAuth2Token()a.appNameGetBearerTokenHeaderTokenStore.GetBearerToken()a.appNameFix: switch every call to the corresponding
ForAppvariant passinga.appName.When
a.appNameis""(no--appflag),ResolveApp("")falls through tothe default app — existing behaviour is fully preserved.
Bug 2 — Named app's credentials were ignored when env vars were set
WithAppNameonly updatedclientID/clientSecretwhen they were alreadyempty. If
CLIENT_ID/CLIENT_SECRETenv vars were set at startup, the namedapp's stored credentials were silently ignored, causing OAuth flows to use the
wrong client application.
Bug 3 — Auth auto-detection in
api/client.goprobed the wrong appWhen no
--authflag is given,getAuthHeaderprobes for available tokensin order (OAuth2 → OAuth1 → Bearer). The existence checks called non-
ForAppvariants, so if the default app had no OAuth2 tokens but the named app did,
the OAuth2 path was skipped entirely — even though the subsequent retrieval
call would have succeeded.
Fix: use
ForAppvariants for the probe checks via a newAuth.AppName()getter, added to expose
appNameto theapipackage without breakingencapsulation.
Bug 4 — OAuth2 tokens saved back to the wrong app (
auth/auth.go)OAuth2FlowandRefreshOAuth2Tokenboth calledSaveOAuth2Token(...)(thenon-
ForAppvariant). This meant that when a user ranxurl --app my-app auth oauth2, the resulting token was written to thedefault app, not
my-app. Refreshed tokens had the same problem.Fix: both methods now call
SaveOAuth2TokenForApp(a.appName, ...).Bug 5 — Token write commands in
cli/auth.goignored--appxurl auth app --bearer-token,xurl auth oauth1, and all variants ofxurl auth clearcalled non-ForAppstore methods. Running any of thesewith
--app my-appwould read from or write to the default app.Fix: all four clear paths and both save paths updated to
ForAppvariantswith
a.AppName().Bug 6 — Webhook CRC validation in
cli/webhook.goignored--appwebhook startfetched the OAuth1 consumer secret (needed to sign CRCchallenge responses) via
TokenStore.GetOAuth1Tokens(), always reading fromthe default app.
Fix:
GetOAuth1TokensForApp(authInstance.AppName()).Files Changed
auth/auth.goGetOAuth1Header:GetOAuth1Tokens()→GetOAuth1TokensForApp(a.appName)GetOAuth2Header: both token lookups useForAppvariants witha.appNameRefreshOAuth2Token: both token lookups useForAppvariants; token save usesSaveOAuth2TokenForApp(a.appName, ...)GetBearerTokenHeader:GetBearerToken()→GetBearerTokenForApp(a.appName)OAuth2Flow:SaveOAuth2Token(...)→SaveOAuth2TokenForApp(a.appName, ...)WithAppName: guard condition flipped so named app credentials override env varsAppName() stringgetter to exposeappNameto theapipackageauth/auth_test.goFour new test functions:
TestWithAppNameOverridesEnvCredentials— verifies that the named app'sstored credentials replace env-var credentials when
--appis used.TestAppFlagTokenIsolation— verifies end-to-end that token lookupsresolve to the correct named app, return errors for missing tokens, and fall
back to the default app when no
--appflag is given (regression guard).TestRefreshOAuth2TokenSavesToNamedApp— uses a mock token server toconfirm that a refreshed OAuth2 token is written back to the named app (not
the default app) when
--appis set.TestRefreshOAuth2TokenSavesToDefaultAppWhenNoOverride— regressionguard confirming that without
--app, refreshed tokens are saved to thedefault app as before.
api/client.gogetAuthHeaderauto-detection probes: both existence checks now useGetFirstOAuth2TokenForApp/GetOAuth1TokensForAppwithc.auth.AppName()api/client_test.goTwo new sub-tests in
TestGetAuthHeader:Auto-detect uses named app bearer token when --app is set— confirmsthe named app's bearer token is selected when the default app has none.
Auto-detect falls back to default app when no --app flag— regressionguard confirming the default app is used when
AppName()returns"".cli/auth.goauth app --bearer-token:SaveBearerToken→SaveBearerTokenForApp(a.AppName(), ...)auth oauth1:SaveOAuth1Tokens→SaveOAuth1TokensForApp(a.AppName(), ...)auth clear --all:ClearAll()→ClearAllForApp(a.AppName())auth clear --oauth1:ClearOAuth1Tokens()→ClearOAuth1TokensForApp(a.AppName())auth clear --oauth2-username:ClearOAuth2Token(...)→ClearOAuth2TokenForApp(a.AppName(), ...)auth clear --bearer:ClearBearerToken()→ClearBearerTokenForApp(a.AppName())cli/webhook.gowebhook start:GetOAuth1Tokens()→GetOAuth1TokensForApp(authInstance.AppName())Behaviour After Fix
xurl whoamixurl whoami --app my-appmy-appxurl /2/users/me --app my-appmy-appxurl --app my-app auth oauth2my-appxurl --app my-app auth clear --allmy-apponlyxurl --app my-app webhook startmy-app's OAuth1 secret for CRC--app