diff --git a/CHANGELOG.md b/CHANGELOG.md index c603b18..3316c5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # CHANGELOG +## Unreleased + +* Don't send the config's `api_key` with the device authorization, token and OIDC audience requests in `hex_api_oauth`. The server refuses a request carrying a credential it refuses, so renewing a refused API token had its refresh or device authorization refused too, and the refused refresh cleared the stored token. +* Renew a credential and run the request again when the server answers with a 401 whose Bearer challenge has `error="invalid_token"`, the RFC 6750 error for an access token that has expired, been revoked or doesn't verify. A stored token is only refreshed or exchanged when it's the one the server rejected, so requests rejected together share one renewal. A refresh that gets no usable answer returns `{error, {auth_error, token_refresh_unavailable}}`, and when the token can't be renewed, a session another request stored in the meantime is used before asking to authenticate again. The `error="token_expired"` challenge is no longer recognized. +* Read the `WWW-Authenticate` header the way RFC 9110 defines it when looking for `invalid_token` and the two-factor `totp_required` and `invalid_totp` errors: the header can hold several challenges, the Bearer challenge's parameters can come in any order with any case, values can be tokens or quoted strings, and `realm` is optional. Only the exact `Bearer realm="hex", error="..."` prefix was recognized before. Add `hex_http:bearer_error/1`. +* When the server answers the `repo_key` from the `get_auth_config` callback with an `invalid_token` 401, `hex_cli_auth:with_repo/2,3` skips that `repo_key` and runs the request again with the parent repository's credentials, the global OAuth token or Workload Identity, instead of returning the 401. The fallback credentials are resolved the way a first request resolves them, so a stored token that hasn't expired is used without a refresh. + ## v0.20.1 (2026-10-14) * Fix `hex_tarball:unpack/2,3` and `hex_tarball:unpack_docs/2,3` looping forever when unpacking to disk a tarball with a GNU sparse member that ends in a hole, and failing on a sparse hole that crosses a 64 KiB boundary in the file, by updating the vendored `erl_tar` to OTP 29.1.2 ([GHSA-phm5-m6cj-5382](https://github.com/erlang/otp/security/advisories/GHSA-phm5-m6cj-5382), CVE-2026-96596). diff --git a/src/hex_api.erl b/src/hex_api.erl index 2327ee3..2c808c4 100644 --- a/src/hex_api.erl +++ b/src/hex_api.erl @@ -173,10 +173,10 @@ to_list(Str) -> unicode:characters_to_list(Str). %% TODO: not needed after exdoc is fixed %% @private detect_otp_error({ok, {401, Headers, Body}}) -> - case maps:get(<<"www-authenticate">>, Headers, nil) of - <<"Bearer realm=\"hex\", error=\"totp_required\"", _/binary>> -> + case hex_http:bearer_error(Headers) of + <<"totp_required">> -> {error, otp_required}; - <<"Bearer realm=\"hex\", error=\"invalid_totp\"", _/binary>> -> + <<"invalid_totp">> -> {error, invalid_totp}; _ -> {ok, {401, Headers, Body}} diff --git a/src/hex_api_oauth.erl b/src/hex_api_oauth.erl index 08351cf..08ca1ce 100644 --- a/src/hex_api_oauth.erl +++ b/src/hex_api_oauth.erl @@ -85,7 +85,7 @@ device_authorization(Config, ClientId, Scope, Opts) -> <<"scope">> => Scope, <<"name">> => Name }, - hex_api:post(Config, Path, Params). + hex_api:post(grant_config(Config), Path, Params). %% @doc %% Runs the complete OAuth device authorization flow. @@ -266,7 +266,7 @@ poll_device_token(Config, ClientId, DeviceCode) -> <<"device_code">> => DeviceCode, <<"client_id">> => ClientId }, - hex_api:post(Config, Path, Params). + hex_api:post(grant_config(Config), Path, Params). %% @doc %% Refreshes an access token using a refresh token. @@ -292,7 +292,7 @@ refresh_token(Config, ClientId, RefreshToken) -> <<"refresh_token">> => RefreshToken, <<"client_id">> => ClientId }, - hex_api:post(Config, Path, Params). + hex_api:post(grant_config(Config), Path, Params). %% @doc %% Requests a browser URL to verify the organizations' SSO and 2FA requirements @@ -373,7 +373,7 @@ client_credentials_token(Config, ClientId, ApiKey, Scope, Opts) -> undefined -> Params0; Name -> Params0#{<<"name">> => Name} end, - hex_api:post(Config, Path, Params). + hex_api:post(grant_config(Config), Path, Params). %% @doc %% Exchanges an OIDC token from a CI provider for an access token scoped to @@ -397,7 +397,7 @@ jwt_bearer_token(Config, Assertion, Scope) -> <<"assertion">> => Assertion, <<"scope">> => Scope }, - hex_api:post(Config, Path, Params). + hex_api:post(grant_config(Config), Path, Params). %% @doc %% Fetches the audience Workload Identity OIDC tokens must be issued for. @@ -412,7 +412,7 @@ jwt_bearer_token(Config, Assertion, Scope) -> %% @end -spec oidc_audience(hex_core:config()) -> hex_api:response(). oidc_audience(Config) -> - hex_api:get(Config, <<"oidc/audience">>). + hex_api:get(grant_config(Config), <<"oidc/audience">>). %% @doc %% Revokes an OAuth token (RFC 7009). @@ -488,6 +488,15 @@ valid_requirements(_) -> %% Internal functions %%==================================================================== +%% @private +%% The device authorization and token endpoints and the OIDC audience take no +%% credential: a grant is authenticated by what's in the request body. hexpm +%% refuses a request whose credential it refuses before it looks at the +%% grant, and the `api_key' in the config is often the token being renewed, +%% so it isn't sent. +grant_config(Config) -> + maps:remove(api_key, Config). + %% @private %% Opens a URL in the default browser using the platform's opener: `open' on %% macOS, `xdg-open' on Linux, `start' on Windows. Returns diff --git a/src/hex_cli_auth.erl b/src/hex_cli_auth.erl index ec9f28d..9a5f04b 100644 --- a/src/hex_cli_auth.erl +++ b/src/hex_cli_auth.erl @@ -145,8 +145,8 @@ %% Maximum OTP retry attempts -define(MAX_OTP_RETRIES, 3). -%% Maximum times a 401 that says the token expired is answered by renewing the -%% credential and running the request again. +%% Maximum times a 401 that says the token is invalid is answered by renewing +%% the credential and running the request again. -define(MAX_TOKEN_RETRIES, 1). %% Both ways a refresh can leave us without a usable token: the server refused @@ -303,15 +303,14 @@ with_api(Permission, BaseConfig, Fun, Opts) -> AuthInline = proplists:get_value(auth_inline, Opts, true), case resolve_api_auth(Permission, BaseConfig) of {ok, ApiKey, AuthContext} -> - Config = BaseConfig#{api_key => ApiKey}, - execute_with_retry(Config, Fun, AuthContext, initial_retries(), Opts); + execute_with_retry(BaseConfig, ApiKey, Fun, AuthContext, initial_retries(), Opts); {error, no_auth} when Optional =:= true -> %% Auth is optional, try without credentials first execute_optional_with_retry(api, BaseConfig, Fun, AuthInline, Opts); {error, no_auth} when AuthInline =:= true -> %% No auth found, ask user if they want to authenticate maybe_authenticate_and_retry( - api, BaseConfig, Fun, no_credentials, initial_retries(), Opts + api, BaseConfig, undefined, Fun, no_credentials, initial_retries(), Opts ); {error, no_auth} -> %% auth_inline is false, just return error @@ -330,7 +329,7 @@ with_api(Permission, BaseConfig, Fun, Opts) -> %% is wrong. Offering a device flow that needs the same network is no %% help. maybe_authenticate_and_retry( - api, BaseConfig, Fun, token_refresh_failed, initial_retries(), Opts + api, BaseConfig, undefined, Fun, token_refresh_failed, initial_retries(), Opts ); {error, _} = Error -> Error @@ -368,9 +367,13 @@ with_repo(BaseConfig, Fun) -> %%
  • Prompt via `should_authenticate' when `auth_inline' is true
  • %% %% -%% A resolved token the server answers with a `token_expired' 401 is renewed at -%% its source (a per-repo or Workload Identity token is exchanged again, the -%% global token is refreshed) and the request is run once more. +%% A resolved token the server answers with a 401 carrying a Bearer +%% challenge with `error="invalid_token"' (RFC 6750 section 3.1) is renewed +%% at its source (a per-repo or Workload Identity token is exchanged again, the +%% global token is refreshed) and the request is run once more. A `repo_key' +%% from `get_auth_config' can't be renewed, so the request is run once more +%% with the credentials that come after it in this order, when there are any. +%% A `repo_key' in config is the caller's choice and its 401 is returned. %% %% The repository name is taken from the config (`repo_name' or `repo_organization'). %% @@ -406,7 +409,7 @@ with_repo(BaseConfig, Fun, Opts) -> no_auth when AuthInline =:= true -> %% No auth found, ask user if they want to authenticate maybe_authenticate_and_retry( - repo, BaseConfig, Fun, no_credentials, initial_retries(), Opts + repo, BaseConfig, undefined, Fun, no_credentials, initial_retries(), Opts ); no_auth -> %% auth_inline is false, return error @@ -447,6 +450,8 @@ repo_name(_) -> %% %% Kind says which credential the retried request needs: `api' takes the token %% as api_key, `repo' resolves repository auth and takes it as repo_key. +%% Rejected is the credential the server refused, or `undefined' when the +%% request had none. %% %% Acquiring the credential is serialized with a global lock so concurrent %% callers don't each trigger their own device auth flow. The first caller to @@ -456,11 +461,11 @@ repo_name(_) -> %% A 401 brings it back here, and global:trans/4 on a lock this process already %% holds does not take a second reference: the inner transaction's exit deletes %% the lock entry while the outer one is still running. -maybe_authenticate_and_retry(Kind, BaseConfig, Fun, Reason, Retries, Opts) -> +maybe_authenticate_and_retry(Kind, BaseConfig, Rejected, Fun, Reason, Retries, Opts) -> Credential = global:trans( {{?MODULE, device_auth}, self()}, fun() -> - acquire_credential(Kind, BaseConfig, Reason, Opts) + acquire_credential(Kind, BaseConfig, Rejected, Reason, Opts) end, [node()], infinity @@ -473,22 +478,20 @@ maybe_authenticate_and_retry(Kind, BaseConfig, Fun, Reason, Retries, Opts) -> end. %% @private -%% Another caller may have authenticated while we waited for the lock. Re-resolve -%% and, if we get a token that differs from the one we arrived with (none when -%% credentials were missing; the rejected one on token_refresh_failed), reuse it -%% instead of prompting again. Otherwise proceed to prompt + device auth. -acquire_credential(api, BaseConfig, Reason, Opts) -> - CurrentApiKey = maps:get(api_key, BaseConfig, undefined), +%% Another caller may have authenticated while we waited for the lock. Resolve +%% again from the caller's config and, if that gives a credential other than the +%% one the server rejected, reuse it instead of prompting again. Otherwise +%% proceed to prompt + device auth. +acquire_credential(api, BaseConfig, Rejected, Reason, Opts) -> case resolve_api_auth(write, BaseConfig) of - {ok, ApiKey, AuthContext} when ApiKey =/= CurrentApiKey -> + {ok, ApiKey, AuthContext} when ApiKey =/= Rejected -> {ok, ApiKey, AuthContext}; _ -> prompt_and_device_auth(api, BaseConfig, Reason, Opts) end; -acquire_credential(repo, BaseConfig, Reason, Opts) -> - CurrentRepoKey = maps:get(repo_key, BaseConfig, undefined), +acquire_credential(repo, BaseConfig, Rejected, Reason, Opts) -> case resolve_repo_auth(BaseConfig) of - {ok, RepoKey, AuthContext} when is_binary(RepoKey), RepoKey =/= CurrentRepoKey -> + {ok, RepoKey, AuthContext} when is_binary(RepoKey), RepoKey =/= Rejected -> {ok, RepoKey, AuthContext}; _ -> prompt_and_device_auth(repo, BaseConfig, Reason, Opts) @@ -526,7 +529,7 @@ authenticated_credential(repo, BaseConfig, _Tokens) -> %% @private execute_authenticated(api, BaseConfig, Fun, ApiKey, AuthContext, Retries, Opts) -> - execute_with_retry(BaseConfig#{api_key => ApiKey}, Fun, AuthContext, Retries, Opts); + execute_with_retry(BaseConfig, ApiKey, Fun, AuthContext, Retries, Opts); execute_authenticated(repo, BaseConfig, Fun, RepoKey, _AuthContext, _Retries, _Opts) -> execute_repo_with_retry(BaseConfig, Fun, RepoKey). @@ -537,7 +540,7 @@ execute_optional_with_retry(Kind, BaseConfig, Fun, AuthInline, Opts) -> {ok, {401, _Headers, _Body}} when AuthInline =:= true -> %% Got 401, need auth - ask user if they want to authenticate maybe_authenticate_and_retry( - Kind, BaseConfig, Fun, no_credentials, initial_retries(), Opts + Kind, BaseConfig, undefined, Fun, no_credentials, initial_retries(), Opts ); {ok, {401, _Headers, _Body}} -> %% Got 401 but auth_inline is false, return error @@ -554,7 +557,7 @@ execute_repo_with_retry(BaseConfig, Fun, RepoKey) -> case Fun(BaseConfig#{repo_key => RepoKey}) of {ok, {401, Headers, _Body}} = Response -> case detect_auth_error(Headers) of - token_expired -> + invalid_token -> renew_repo_auth_and_retry(BaseConfig, Fun, RepoKey, Response); _Other -> Response @@ -731,7 +734,7 @@ resolve_api_auth(_Permission, Config) -> %% @private %% Resolve repo auth credentials in this order: %% 0. repo_key in config => passthrough -%% 1. repo_key from get_auth_config => passthrough +%% 1. repo_key from get_auth_config, unless the server rejected it => passthrough %% 2. trusted + auth_key + oauth_exchange => exchange for OAuth token %% 3. trusted + auth_key => use directly %% 4. trusted Hex.pm or child repository + global OAuth tokens => use those @@ -744,10 +747,14 @@ resolve_repo_auth(Config) -> %% @private %% Renew is `{rejected, RepoKey}' when the server rejected the credential we -%% have, so a stored token that has not run out of time is exchanged or -%% refreshed anyway, and `false' otherwise. +%% have, and `false' otherwise. A stored token that is the rejected one is +%% exchanged or refreshed even though it has not run out of time, and one that +%% isn't, because another caller already renewed it or because it comes after +%% a rejected `repo_key' from `get_auth_config', is used as it is. A rejected +%% `repo_key' can't be renewed, so it's skipped, see skip_rejected_repo_key/2. resolve_repo_auth(#{repo_key := RepoKey}, _Renew) when is_binary(RepoKey) -> - %% repo_key already in config, pass through directly + %% repo_key already in config, pass through directly. The caller chose it + %% for this request, so a rejection is the caller's to handle. {ok, RepoKey, #{has_refresh_token => false}}; resolve_repo_auth(Config, Renew) -> RepoName = repo_name(Config), @@ -772,9 +779,12 @@ resolve_repo_auth(Config, Renew) -> do_resolve_repo_auth(RepoName, LookupRepo, Config, Renew, Lock) -> Trusted = maps:get(trusted, Config, false), OAuthExchange = maps:get(oauth_exchange, Config, false), - case call_callback(Config, get_auth_config, [LookupRepo]) of + AuthConfig = skip_rejected_repo_key( + call_callback(Config, get_auth_config, [LookupRepo]), Renew + ), + case AuthConfig of #{repo_key := RepoKey} when is_binary(RepoKey) -> - %% 1. repo_key from get_auth_config => passthrough + %% 1. repo_key from get_auth_config, unless the server rejected it => passthrough {ok, RepoKey, #{has_refresh_token => false}}; #{oauth_token := OAuthToken, auth_key := AuthKey} when is_binary(AuthKey) and OAuthExchange, Trusted @@ -810,6 +820,21 @@ do_resolve_repo_auth(RepoName, LookupRepo, Config, Renew, Lock) -> no_auth end. +%% @private +%% A repo_key the server rejected is skipped, and the resolution continues as +%% if the build tool hadn't returned it, for the parent repository too. +skip_rejected_repo_key(#{repo_key := RepoKey} = AuthConfig, {rejected, RepoKey}) -> + maps:remove(repo_key, AuthConfig); +skip_rejected_repo_key(AuthConfig, _Renew) -> + AuthConfig. + +%% @private +%% Whether the stored token is the credential the server rejected. +is_rejected({rejected, BearerToken}, #{access_token := AccessToken}) -> + BearerToken =:= <<"Bearer ", AccessToken/binary>>; +is_rejected(false, _Tokens) -> + false. + %% @private resolve_global_oauth_for_repo(<<"hexpm">>, Config, Renew) -> resolve_global_oauth_for_repo(Config, Renew); @@ -819,7 +844,7 @@ resolve_global_oauth_for_repo(_RepoName, _Config, _Renew) -> no_auth. resolve_global_oauth_for_repo(Config, Renew) -> - case resolve_oauth_token_with_context(Config, Renew =/= false) of + case resolve_oauth_token_with_context(Config, Renew) of {ok, Token, AuthContext} -> {ok, Token, AuthContext}; {error, no_auth} -> @@ -898,11 +923,11 @@ resolve_repo_oauth_token( RepoName, Config, AuthKey, - #{access_token := AccessToken, expires_at := ExpiresAt}, + #{access_token := AccessToken, expires_at := ExpiresAt} = OAuthToken, Renew, Lock ) -> - case {Renew =/= false orelse is_token_expired(ExpiresAt), Lock} of + case {is_rejected(Renew, OAuthToken) orelse is_token_expired(ExpiresAt), Lock} of {false, _} -> %% Token is still valid, use it BearerToken = <<"Bearer ", AccessToken/binary>>, @@ -959,12 +984,14 @@ get_parent_repo_key(Config, RepoName, KeyType) -> %% @private %% Resolve OAuth token with global lock to prevent concurrent refresh attempts. -%% Renew refreshes a token that has not run out of time, for when the server -%% has rejected it anyway. +%% Renew is `{rejected, BearerToken}' when the server rejected a token, which +%% refreshes the stored token if it's that one, even though it has not run out +%% of time. %% %% A stored token that is still valid, or no stored token, is answered without -%% the lock. Only a refresh takes it, and the tokens are read again under the -%% lock in case another caller refreshed them while this one waited. +%% the lock. A renewal or a refresh takes it, and the tokens are read again +%% under the lock, so a caller that waited for another caller's refresh uses +%% the refreshed token instead of refreshing again. resolve_oauth_token_with_context(Config, false) -> case call_callback(Config, get_oauth_tokens, []) of {ok, #{expires_at := ExpiresAt} = Tokens} -> @@ -975,12 +1002,12 @@ resolve_oauth_token_with_context(Config, false) -> error -> {error, no_auth} end; -resolve_oauth_token_with_context(Config, true) -> - resolve_oauth_token_with_lock(Config, true). +resolve_oauth_token_with_context(Config, {rejected, _BearerToken} = Renew) -> + resolve_oauth_token_with_lock(Config, Renew). resolve_oauth_token_with_lock(Config, Renew) -> Resolve = fun(#{expires_at := ExpiresAt} = Tokens) -> - case Renew orelse is_token_expired(ExpiresAt) of + case is_rejected(Renew, Tokens) orelse is_token_expired(ExpiresAt) of true -> refresh_or_clear(Config, Tokens); false -> bearer_token(Tokens) end @@ -1103,15 +1130,19 @@ initial_retries() -> #{otp => 0, otp_error => undefined, token => 0}. %% @private -execute_with_retry(Config, Fun, AuthContext, Retries, Opts) -> - case Fun(Config) of +%% Runs Fun with ApiKey. BaseConfig is the caller's config, kept apart from the +%% credential so a renewal resolves from what the caller passed rather than +%% from the key that was rejected. +execute_with_retry(BaseConfig, ApiKey, Fun, AuthContext, Retries, Opts) -> + case Fun(BaseConfig#{api_key => ApiKey}) of {error, otp_required} -> handle_otp_retry( - Config, Fun, AuthContext, Retries, <<"Enter OTP code:">>, Opts + BaseConfig, ApiKey, Fun, AuthContext, Retries, <<"Enter OTP code:">>, Opts ); {error, invalid_totp} -> handle_otp_retry( - Config, + BaseConfig, + ApiKey, Fun, AuthContext, Retries, @@ -1122,7 +1153,7 @@ execute_with_retry(Config, Fun, AuthContext, Retries, Opts) -> case detect_auth_error(Headers) of otp_required -> handle_otp_retry( - Config, Fun, AuthContext, Retries, <<"Enter OTP code:">>, Opts + BaseConfig, ApiKey, Fun, AuthContext, Retries, <<"Enter OTP code:">>, Opts ); invalid_totp -> Msg = @@ -1130,9 +1161,11 @@ execute_with_retry(Config, Fun, AuthContext, Retries, Opts) -> invalid_totp -> <<"Invalid OTP code. Please try again:">>; _ -> <<"Enter OTP code:">> end, - handle_otp_retry(Config, Fun, AuthContext, Retries, Msg, Opts); - token_expired -> - handle_token_refresh_retry(Config, Fun, AuthContext, Retries, Response, Opts); + handle_otp_retry(BaseConfig, ApiKey, Fun, AuthContext, Retries, Msg, Opts); + invalid_token -> + handle_token_refresh_retry( + BaseConfig, ApiKey, Fun, AuthContext, Retries, Response, Opts + ); none -> Response end; @@ -1141,82 +1174,81 @@ execute_with_retry(Config, Fun, AuthContext, Retries, Opts) -> end. %% @private -handle_otp_retry(_Config, _Fun, _AuthContext, #{otp := OtpRetries}, _Message, _Opts) when +handle_otp_retry( + _BaseConfig, _ApiKey, _Fun, _AuthContext, #{otp := OtpRetries}, _Message, _Opts +) when OtpRetries >= ?MAX_OTP_RETRIES -> {error, {auth_error, otp_max_retries}}; -handle_otp_retry(Config, Fun, AuthContext, #{otp := OtpRetries} = Retries, Message, Opts) -> - case call_callback(Config, prompt_otp, [Message]) of +handle_otp_retry( + BaseConfig, ApiKey, Fun, AuthContext, #{otp := OtpRetries} = Retries, Message, Opts +) -> + case call_callback(BaseConfig, prompt_otp, [Message]) of {ok, OtpCode} -> - NewConfig = Config#{api_otp => OtpCode}, + NewConfig = BaseConfig#{api_otp => OtpCode}, NewRetries = Retries#{otp := OtpRetries + 1, otp_error := invalid_totp}, - execute_with_retry(NewConfig, Fun, AuthContext, NewRetries, Opts); + execute_with_retry(NewConfig, ApiKey, Fun, AuthContext, NewRetries, Opts); cancelled -> {error, {auth_error, otp_cancelled}} end. %% @private -%% A 401 that says the token expired is answered by renewing the credential at -%% its source and running the request once more, the way a repository request -%% is. The renewal is counted, so a server that answers token_expired to every +%% A 401 that says the token is invalid is answered by renewing the credential +%% at its source and running the request once more, the way a repository request +%% is. The renewal is counted, so a server that answers invalid_token to every %% bearer we send it gets a bounded number of requests rather than a loop. +%% +%% A refresh that got no usable answer says nothing about the token, so it is +%% returned rather than offering to authenticate again over the same network. handle_token_refresh_retry( - _Config, _Fun, _AuthContext, #{token := TokenRetries}, Response, _Opts + _BaseConfig, _ApiKey, _Fun, _AuthContext, #{token := TokenRetries}, Response, _Opts ) when TokenRetries >= ?MAX_TOKEN_RETRIES -> Response; handle_token_refresh_retry( - Config, Fun, AuthContext, #{token := TokenRetries} = Retries, _Response, Opts + BaseConfig, ApiKey, Fun, AuthContext, #{token := TokenRetries} = Retries, _Response, Opts ) -> NewRetries = Retries#{token := TokenRetries + 1}, %% Only attempt refresh if we have a refresh token case maps:get(has_refresh_token, AuthContext, false) of true -> - case resolve_oauth_token_with_context(Config, true) of + case resolve_oauth_token_with_context(BaseConfig, {rejected, ApiKey}) of {ok, NewBearerToken, NewAuthContext} -> - NewConfig = Config#{api_key => NewBearerToken}, - execute_with_retry(NewConfig, Fun, NewAuthContext, NewRetries, Opts); + execute_with_retry( + BaseConfig, NewBearerToken, Fun, NewAuthContext, NewRetries, Opts + ); + {error, {auth_error, token_refresh_unavailable}} = Error -> + Error; {error, _} -> - maybe_reauthenticate(Config, Fun, NewRetries, Opts) + maybe_reauthenticate(BaseConfig, ApiKey, Fun, NewRetries, Opts) end; false -> - maybe_reauthenticate(Config, Fun, NewRetries, Opts) + maybe_reauthenticate(BaseConfig, ApiKey, Fun, NewRetries, Opts) end. %% @private %% After token refresh failure, prompt the user to re-authenticate via device auth %% (only when auth_inline is true). Mirrors Hex.OAuth.reauthenticate/1. -maybe_reauthenticate(Config, Fun, Retries, Opts) -> +maybe_reauthenticate(BaseConfig, ApiKey, Fun, Retries, Opts) -> AuthInline = proplists:get_value(auth_inline, Opts, true), case AuthInline of true -> - maybe_authenticate_and_retry(api, Config, Fun, token_refresh_failed, Retries, Opts); + maybe_authenticate_and_retry( + api, BaseConfig, ApiKey, Fun, token_refresh_failed, Retries, Opts + ); false -> {error, {auth_error, token_refresh_failed}} end. %% @private --spec detect_auth_error(hex_http:headers()) -> otp_required | invalid_totp | token_expired | none. +-spec detect_auth_error(hex_http:headers()) -> otp_required | invalid_totp | invalid_token | none. detect_auth_error(Headers) -> - case maps:get(<<"www-authenticate">>, Headers, undefined) of - undefined -> - none; - Value -> - parse_www_authenticate(Value) - end. - -%% @private -parse_www_authenticate(Value) when is_binary(Value) -> - case Value of - <<"Bearer realm=\"hex\", error=\"totp_required\"", _/binary>> -> - otp_required; - <<"Bearer realm=\"hex\", error=\"invalid_totp\"", _/binary>> -> - invalid_totp; - <<"Bearer realm=\"hex\", error=\"token_expired\"", _/binary>> -> - token_expired; - _ -> - none + case hex_http:bearer_error(Headers) of + <<"totp_required">> -> otp_required; + <<"invalid_totp">> -> invalid_totp; + <<"invalid_token">> -> invalid_token; + _ -> none end. %%==================================================================== diff --git a/src/hex_http.erl b/src/hex_http.erl index ed95b46..64482a9 100644 --- a/src/hex_http.erl +++ b/src/hex_http.erl @@ -1,7 +1,7 @@ %% @doc %% HTTP contract. -module(hex_http). --export([request/5, request_to_file/6]). +-export([request/5, request_to_file/6, bearer_error/1]). -ifdef(TEST). -export([user_agent/1]). -endif. @@ -46,6 +46,27 @@ request_to_file(Config, Method, URI, Headers, Body, Filename) when Headers2 = put_new(<<"user-agent">>, user_agent(UserAgentFragment), Headers), Adapter:request_to_file(Method, URI, Headers2, Body, Filename, AdapterConfig). +%% @doc +%% The `error' parameter of the first Bearer challenge that has one in a +%% response's `www-authenticate' header (RFC 6750 section 3), or `undefined' +%% when there is none. +%% +%% The header is read the way RFC 9110 section 11.6.1 defines it: it can hold +%% several challenges, a scheme can be followed by a token68 or by +%% parameters, scheme and parameter names are case-insensitive, and a +%% parameter value is a token or a quoted string. +-spec bearer_error(headers()) -> binary() | undefined. +bearer_error(Headers) -> + case maps:get(<<"www-authenticate">>, Headers, undefined) of + Value when is_binary(Value) -> + case [Error || {<<"bearer">>, #{<<"error">> := Error}} <- parse_challenges(Value)] of + [Error | _] -> Error; + [] -> undefined + end; + undefined -> + undefined + end. + %% @private user_agent(UserAgentFragment) -> OTPRelease = erlang:system_info(otp_release), @@ -78,3 +99,145 @@ put_new(Key, Value, Map) -> {ok, _} -> Map; error -> maps:put(Key, Value, Map) end. + +%%==================================================================== +%% Internal functions - WWW-Authenticate +%%==================================================================== + +%% @private +%% WWW-Authenticate = #challenge +%% challenge = auth-scheme [ 1*SP ( token68 / #auth-param ) ] +%% +%% Parsing stops at the first thing that isn't part of a challenge and +%% returns the challenges read up to there. +parse_challenges(Value) -> + parse_challenges(skip_separators(Value), []). + +parse_challenges(Value, Acc) -> + case take_token(Value) of + {<<>>, _Rest} -> + lists:reverse(Acc); + {Scheme, Rest} -> + {Params, Rest1} = parse_challenge_params(Rest), + parse_challenges(skip_separators(Rest1), [{lowercase(Scheme), Params} | Acc]) + end. + +%% A scheme is followed by a space and either a token68 or parameters, or +%% by nothing. A token68 ends the challenge, so it is followed by a comma or +%% the end of the field. +parse_challenge_params(<> = Value) when C =:= $\s -> + Value1 = skip_ows(Value), + case take_token68(Value1) of + {<<_, _/binary>>, Rest} -> + case skip_ows(Rest) of + <<>> -> {#{}, <<>>}; + <<$,, _/binary>> = Rest1 -> {#{}, Rest1}; + _ -> parse_params(Value1, #{}) + end; + {<<>>, _Rest} -> + %% The parameter list can start with empty elements + parse_params(skip_separators(Value1), #{}) + end; +parse_challenge_params(Value) -> + {#{}, Value}. + +%% auth-param = token BWS "=" BWS ( token / quoted-string ) +%% +%% A token that isn't followed by "=" is the scheme of the next challenge. +parse_params(Value, Acc) -> + case take_token(Value) of + {<<>>, _Rest} -> + {Acc, Value}; + {Name, Rest} -> + case skip_ows(Rest) of + <<$=, Rest1/binary>> -> + case take_param_value(skip_ows(Rest1)) of + {ok, ParamValue, Rest2} -> + Acc1 = put_new(lowercase(Name), ParamValue, Acc), + next_param(skip_ows(Rest2), Acc1); + error -> + {Acc, <<>>} + end; + _ -> + {Acc, Value} + end + end. + +next_param(<<$,, Rest/binary>>, Acc) -> + parse_params(skip_separators(Rest), Acc); +next_param(Value, Acc) -> + {Acc, Value}. + +take_param_value(<<$", Rest/binary>>) -> + take_quoted_string(Rest, <<>>); +take_param_value(Value) -> + case take_token(Value) of + {<<>>, _Rest} -> error; + {Token, Rest} -> {ok, Token, Rest} + end. + +%% quoted-string = DQUOTE *( qdtext / quoted-pair ) DQUOTE +take_quoted_string(<<$", Rest/binary>>, Acc) -> + {ok, Acc, Rest}; +take_quoted_string(<<$\\, C, Rest/binary>>, Acc) -> + take_quoted_string(Rest, <>); +take_quoted_string(<>, Acc) when C =/= $\\ -> + take_quoted_string(Rest, <>); +take_quoted_string(_Value, _Acc) -> + error. + +take_token(Value) -> + take_while(fun is_tchar/1, Value). + +%% token68 = 1*( ALPHA / DIGIT / "-" / "." / "_" / "~" / "+" / "/" ) *"=" +take_token68(Value) -> + case take_while(fun is_token68_char/1, Value) of + {<<>>, _Rest} = Empty -> + Empty; + {Token, Rest} -> + {Padding, Rest1} = take_while(fun(C) -> C =:= $= end, Rest), + {<>, Rest1} + end. + +take_while(Pred, Value) -> + take_while(Pred, Value, 0). + +take_while(Pred, Value, N) -> + case Value of + <<_:N/binary, C, _/binary>> -> + case Pred(C) of + true -> take_while(Pred, Value, N + 1); + false -> split_binary(Value, N) + end; + _ -> + split_binary(Value, N) + end. + +%% Commas and whitespace between the elements of a list, which can also be +%% empty (RFC 9110 section 5.6.1). +skip_separators(<>) when C =:= $\s; C =:= $\t; C =:= $, -> + skip_separators(Rest); +skip_separators(Value) -> + Value. + +skip_ows(<>) when C =:= $\s; C =:= $\t -> + skip_ows(Rest); +skip_ows(Value) -> + Value. + +is_tchar(C) when C >= $a, C =< $z -> true; +is_tchar(C) when C >= $A, C =< $Z -> true; +is_tchar(C) when C >= $0, C =< $9 -> true; +is_tchar(C) -> lists:member(C, "!#$%&'*+-.^_`|~"). + +is_token68_char(C) when C >= $a, C =< $z -> true; +is_token68_char(C) when C >= $A, C =< $Z -> true; +is_token68_char(C) when C >= $0, C =< $9 -> true; +is_token68_char(C) -> lists:member(C, "-._~+/"). + +%% Scheme and parameter names are tokens, which are ASCII. +lowercase(Value) -> + <<<<(ascii_lowercase(C))>> || <> <= Value>>. + +ascii_lowercase(C) when C >= $A, C =< $Z -> C + ($a - $A); +ascii_lowercase(C) -> C. diff --git a/test/hex_cli_auth_SUITE.erl b/test/hex_cli_auth_SUITE.erl index 2fa335c..fcfcff8 100644 --- a/test/hex_cli_auth_SUITE.erl +++ b/test/hex_cli_auth_SUITE.erl @@ -64,15 +64,18 @@ all() -> refresh_tokens_without_credentials_test, %% with_api tests - token refresh on 401 - with_api_token_expired_refresh_test, - with_api_token_expired_renews_test, - with_api_token_expired_retry_bounded_test, - with_api_token_expired_reauth_retry_bounded_test, + with_api_invalid_token_refresh_test, + with_api_invalid_token_renews_test, + with_api_invalid_token_retry_bounded_test, + with_api_invalid_token_reauth_retry_bounded_test, + with_api_invalid_token_refreshed_elsewhere_test, + with_api_invalid_token_replaced_session_test, + with_api_invalid_token_refresh_unavailable_test, %% with_api tests - reauthentication after refresh failure - with_api_token_expired_reauth_yes_test, - with_api_token_expired_reauth_no_test, - with_api_token_expired_reauth_inline_false_test, + with_api_invalid_token_reauth_yes_test, + with_api_invalid_token_reauth_no_test, + with_api_invalid_token_reauth_inline_false_test, %% with_api tests - wrapper behavior with_api_optional_test, @@ -87,8 +90,15 @@ all() -> with_repo_optional_token_refresh_failed_test, with_repo_optional_401_does_not_prompt_test, with_repo_device_auth_sets_repo_key_test, - with_repo_token_expired_refresh_test, - with_repo_token_expired_exchange_test, + with_repo_invalid_token_refresh_test, + with_repo_invalid_token_exchange_test, + with_repo_rejected_repo_key_oauth_test, + with_repo_rejected_repo_key_same_token_test, + with_repo_rejected_repo_key_no_fallback_test, + with_repo_rejected_config_repo_key_test, + with_repo_insufficient_scope_test, + with_repo_repo_key_before_oauth_test, + with_repo_replaced_repo_key_test, %% token refresh failure modes refresh_transport_error_keeps_token_test, @@ -133,9 +143,10 @@ groups() -> with_repo_workload_identity_failed_test, with_repo_workload_identity_failed_unauthenticated_test, with_repo_workload_identity_failed_required_test, - with_repo_token_expired_workload_identity_test, - with_repo_token_expired_workload_identity_renewed_test, - with_repo_token_expired_workload_identity_failed_test + with_repo_invalid_token_workload_identity_test, + with_repo_invalid_token_workload_identity_renewed_test, + with_repo_invalid_token_workload_identity_failed_test, + with_repo_rejected_repo_key_workload_identity_test ]} ]. @@ -619,8 +630,8 @@ with_api_otp_max_retries_test(_Config) -> %% Test Cases - with_api token refresh on 401 %%==================================================================== -with_api_token_expired_refresh_test(_Config) -> - %% Test token refresh when server returns token_expired +with_api_invalid_token_refresh_test(_Config) -> + %% Test token refresh when server returns invalid_token Now = erlang:system_time(second), Self = self(), Config = config_with_callbacks(#{ @@ -654,7 +665,7 @@ with_api_token_expired_refresh_test(_Config) -> {401, #{ <<"www-authenticate">> => - <<"Bearer realm=\"hex\", error=\"token_expired\"">> + <<"Bearer realm=\"hex\", error=\"invalid_token\"">> }, <<>>}}; _ -> @@ -675,8 +686,8 @@ with_api_token_expired_refresh_test(_Config) -> end, ok. -with_api_token_expired_renews_test(_Config) -> - %% An API token the server rejects as expired is refreshed even though its +with_api_invalid_token_renews_test(_Config) -> + %% An API token the server rejects as invalid is refreshed even though its %% stored expiry has not passed, the way a repository token is, and the %% request runs again with the token that came back. Now = erlang:system_time(second), @@ -693,7 +704,7 @@ with_api_token_expired_renews_test(_Config) -> Fun = fun(Cfg) -> case maps:get(api_key, Cfg) of - <<"Bearer stale_token">> -> token_expired_response(); + <<"Bearer stale_token">> -> invalid_token_response(); ApiKey -> {ok, {200, #{}, ApiKey}} end end, @@ -704,8 +715,8 @@ with_api_token_expired_renews_test(_Config) -> ), ok. -with_api_token_expired_retry_bounded_test(_Config) -> - %% A server that answers token_expired to every bearer it is sent gets a +with_api_invalid_token_retry_bounded_test(_Config) -> + %% A server that answers invalid_token to every bearer it is sent gets a %% bounded number of requests: the renewed token is tried once and the 401 %% is handed back, rather than renewed and retried without end. Now = erlang:system_time(second), @@ -722,17 +733,17 @@ with_api_token_expired_retry_bounded_test(_Config) -> CallCount = counters:new(1, []), Result = hex_cli_auth:with_api(write, Config, fun(_Cfg) -> counters:add(CallCount, 1, 1), - token_expired_response() + invalid_token_response() end), ?assertMatch({ok, {401, _Headers, _Body}}, Result), ?assertEqual(2, counters:get(CallCount, 1)), ok. -with_api_token_expired_reauth_retry_bounded_test(_Config) -> +with_api_invalid_token_reauth_retry_bounded_test(_Config) -> %% Same bound when the renewal is a device auth: the user authenticates %% once, the request runs again with the new token, and a second - %% token_expired is the caller's to handle rather than a second prompt. + %% invalid_token is the caller's to handle rather than a second prompt. Now = erlang:system_time(second), PromptCount = counters:new(1, []), CallCount = counters:new(1, []), @@ -756,7 +767,7 @@ with_api_token_expired_reauth_retry_bounded_test(_Config) -> Config, fun(_Cfg) -> counters:add(CallCount, 1, 1), - token_expired_response() + invalid_token_response() end, [{oauth_open_browser, false}] ), @@ -770,7 +781,95 @@ with_api_token_expired_reauth_retry_bounded_test(_Config) -> %% Test Cases - with_api reauthentication after refresh failure %%==================================================================== -with_api_token_expired_reauth_yes_test(_Config) -> +with_api_invalid_token_refreshed_elsewhere_test(_Config) -> + %% A token another request refreshed while this one waited for the lock is + %% used without refreshing it again. + Now = erlang:system_time(second), + Reads = counters:new(1, []), + Config = config_with_callbacks(#{ + get_oauth_tokens => fun() -> + counters:add(Reads, 1, 1), + AccessToken = + case counters:get(Reads, 1) of + 1 -> <<"stale_token">>; + _ -> <<"refreshed_token">> + end, + {ok, #{ + access_token => AccessToken, + refresh_token => <<"refresh_token">>, + expires_at => Now + 3600 + }} + end, + persist_oauth_tokens => fun(_Scope, _Access, _Refresh, _Expires) -> + error(refreshed_again) + end + }), + + Fun = fun(Cfg) -> + case maps:get(api_key, Cfg) of + <<"Bearer stale_token">> -> invalid_token_response(); + ApiKey -> ApiKey + end + end, + + ?assertEqual(<<"Bearer refreshed_token">>, hex_cli_auth:with_api(read, Config, Fun)), + ok. + +with_api_invalid_token_replaced_session_test(_Config) -> + %% A session that can't be refreshed and was replaced by another request + %% is used instead of asking to authenticate again. + Now = erlang:system_time(second), + Reads = counters:new(1, []), + Config = config_with_callbacks(#{ + get_oauth_tokens => fun() -> + counters:add(Reads, 1, 1), + AccessToken = + case counters:get(Reads, 1) of + 1 -> <<"stale_token">>; + _ -> <<"replaced_token">> + end, + {ok, #{access_token => AccessToken, expires_at => Now + 3600}} + end, + should_authenticate => fun(_Reason) -> error(prompted) end + }), + + Fun = fun(Cfg) -> + case maps:get(api_key, Cfg) of + <<"Bearer stale_token">> -> invalid_token_response(); + ApiKey -> ApiKey + end + end, + + ?assertEqual(<<"Bearer replaced_token">>, hex_cli_auth:with_api(read, Config, Fun)), + ok. + +with_api_invalid_token_refresh_unavailable_test(_Config) -> + %% A refresh that got no usable answer says nothing about the token, so + %% it's returned rather than answered with an offer to authenticate again. + Now = erlang:system_time(second), + Config = config_with_callbacks(#{ + oauth_tokens => + {ok, #{ + access_token => <<"stale_token">>, + refresh_token => <<"refresh_token">>, + expires_at => Now + 3600 + }}, + should_authenticate => fun(_Reason) -> error(prompted) end + }), + self() ! {hex_http_test, oauth_refresh_response, {ok, {503, #{}, <<"">>}}}, + + Fun = fun(Cfg) -> + <<"Bearer stale_token">> = maps:get(api_key, Cfg), + invalid_token_response() + end, + + ?assertEqual( + {error, {auth_error, token_refresh_unavailable}}, + hex_cli_auth:with_api(read, Config, Fun) + ), + ok. + +with_api_invalid_token_reauth_yes_test(_Config) -> %% When token refresh fails and user agrees to re-authenticate, %% device auth flow is triggered and the operation retried. Self = self(), @@ -780,7 +879,7 @@ with_api_token_expired_reauth_yes_test(_Config) -> {ok, #{ access_token => <<"initial_token">>, %% No refresh_token; token is valid so it's used, but server - %% returns token_expired 401, triggering the reauth path + %% returns invalid_token 401, triggering the reauth path expires_at => Now + 3600 }}, should_authenticate => fun(token_refresh_failed) -> @@ -819,7 +918,7 @@ with_api_token_expired_reauth_yes_test(_Config) -> {401, #{ <<"www-authenticate">> => - <<"Bearer realm=\"hex\", error=\"token_expired\"">> + <<"Bearer realm=\"hex\", error=\"invalid_token\"">> }, <<>>}}; _ -> @@ -843,7 +942,7 @@ with_api_token_expired_reauth_yes_test(_Config) -> end, ok. -with_api_token_expired_reauth_no_test(_Config) -> +with_api_invalid_token_reauth_no_test(_Config) -> %% When token refresh fails and user declines to re-authenticate, %% returns auth_declined error. Self = self(), @@ -853,7 +952,7 @@ with_api_token_expired_reauth_no_test(_Config) -> {ok, #{ access_token => <<"initial_token">>, %% No refresh_token; token is valid so it's used, but server - %% returns token_expired 401, triggering the reauth path + %% returns invalid_token 401, triggering the reauth path expires_at => Now + 3600 }}, should_authenticate => fun(token_refresh_failed) -> @@ -870,7 +969,7 @@ with_api_token_expired_reauth_no_test(_Config) -> {401, #{ <<"www-authenticate">> => - <<"Bearer realm=\"hex\", error=\"token_expired\"">> + <<"Bearer realm=\"hex\", error=\"invalid_token\"">> }, <<>>}} end @@ -884,7 +983,7 @@ with_api_token_expired_reauth_no_test(_Config) -> end, ok. -with_api_token_expired_reauth_inline_false_test(_Config) -> +with_api_invalid_token_reauth_inline_false_test(_Config) -> %% When auth_inline is false, token refresh failure returns error directly %% without prompting the user. Now = erlang:system_time(second), @@ -893,7 +992,7 @@ with_api_token_expired_reauth_inline_false_test(_Config) -> {ok, #{ access_token => <<"initial_token">>, %% No refresh_token; token is valid so it's used, but server - %% returns token_expired 401, triggering the reauth path + %% returns invalid_token 401, triggering the reauth path expires_at => Now + 3600 }}, should_authenticate => fun(_) -> error(should_not_be_called) end @@ -907,7 +1006,7 @@ with_api_token_expired_reauth_inline_false_test(_Config) -> {401, #{ <<"www-authenticate">> => - <<"Bearer realm=\"hex\", error=\"token_expired\"">> + <<"Bearer realm=\"hex\", error=\"invalid_token\"">> }, <<>>}} end, @@ -1163,8 +1262,8 @@ with_repo_device_auth_sets_repo_key_test(_Config) -> ets:delete(TokenStore), ok. -with_repo_token_expired_refresh_test(_Config) -> - %% A repository token the server rejects as expired is refreshed and the +with_repo_invalid_token_refresh_test(_Config) -> + %% A repository token the server rejects as invalid is refreshed and the %% request runs again, rather than the 401 reaching the caller. Now = erlang:system_time(second), Config = config_with_callbacks(#{ @@ -1180,7 +1279,7 @@ with_repo_token_expired_refresh_test(_Config) -> Fun = fun(Cfg) -> case maps:get(repo_key, Cfg) of - <<"Bearer stale_token">> -> token_expired_response(); + <<"Bearer stale_token">> -> invalid_token_response(); RepoKey -> RepoKey end end, @@ -1189,7 +1288,7 @@ with_repo_token_expired_refresh_test(_Config) -> ?assertEqual(<<"Bearer renewed_token">>, Result), ok. -with_repo_token_expired_exchange_test(_Config) -> +with_repo_invalid_token_exchange_test(_Config) -> %% Same for a per-repo token: it is exchanged again from the auth_key it %% came from, even though its stored expiry has not passed. Now = erlang:system_time(second), @@ -1212,7 +1311,7 @@ with_repo_token_expired_exchange_test(_Config) -> Fun = fun(Cfg) -> case maps:get(repo_key, Cfg) of - <<"Bearer stale_repo_token">> -> token_expired_response(); + <<"Bearer stale_repo_token">> -> invalid_token_response(); RepoKey -> RepoKey end end, @@ -1229,6 +1328,206 @@ with_repo_token_expired_exchange_test(_Config) -> end, ok. +with_repo_rejected_repo_key_oauth_test(_Config) -> + %% A repo_key from get_auth_config can't be renewed, so after the server + %% rejects it the request runs again with the session. The build tool hands + %% back the same repo_key for the parent repository, which is skipped too. + %% The session token isn't what the server rejected, so it's used without + %% a refresh, which would have replaced it with a random token. + Now = erlang:system_time(second), + OrganizationAuth = #{repo_key => <<"Bearer org_token">>}, + Config = config_with_callbacks(#{ + auth_config => #{<<"hexpm:acme">> => OrganizationAuth, <<"hexpm">> => OrganizationAuth}, + oauth_tokens => + {ok, #{ + access_token => <<"global_oauth">>, + refresh_token => <<"refresh_token">>, + expires_at => Now + 3600 + }} + }), + + Fun = fun(Cfg) -> + case maps:get(repo_key, Cfg) of + <<"Bearer org_token">> -> invalid_token_response(); + RepoKey -> RepoKey + end + end, + + ?assertEqual( + <<"Bearer global_oauth">>, + hex_cli_auth:with_repo(Config#{repo_organization => <<"acme">>, trusted => true}, Fun) + ), + ok. + +with_repo_rejected_repo_key_same_token_test(_Config) -> + %% A fallback credential that is the token the server rejected is renewed + %% rather than sent again. + Now = erlang:system_time(second), + OrganizationAuth = #{repo_key => <<"Bearer same_token">>}, + Config = config_with_callbacks(#{ + auth_config => #{<<"hexpm:acme">> => OrganizationAuth, <<"hexpm">> => OrganizationAuth}, + oauth_tokens => + {ok, #{ + access_token => <<"same_token">>, + refresh_token => <<"refresh_token">>, + expires_at => Now + 3600 + }} + }), + queue_refresh_response(#{<<"access_token">> => <<"renewed_token">>}), + + Fun = fun(Cfg) -> + case maps:get(repo_key, Cfg) of + <<"Bearer same_token">> -> invalid_token_response(); + RepoKey -> RepoKey + end + end, + + ?assertEqual( + <<"Bearer renewed_token">>, + hex_cli_auth:with_repo(Config#{repo_organization => <<"acme">>, trusted => true}, Fun) + ), + ok. + +with_repo_rejected_repo_key_no_fallback_test(_Config) -> + %% With nothing after the rejected repo_key, the rejection is returned + %% after one request instead of the request running again with the same + %% repo_key. + Requests = counters:new(1, []), + Config = config_with_callbacks(#{ + auth_config => #{<<"hexpm:acme">> => #{repo_key => <<"Bearer org_token">>}}, + oauth_tokens => error + }), + + Fun = fun(Cfg) -> + counters:add(Requests, 1, 1), + <<"Bearer org_token">> = maps:get(repo_key, Cfg), + invalid_token_response() + end, + + ?assertEqual( + invalid_token_response(), + hex_cli_auth:with_repo(Config#{repo_organization => <<"acme">>, trusted => true}, Fun) + ), + ?assertEqual(1, counters:get(Requests, 1)), + ok. + +with_repo_insufficient_scope_test(_Config) -> + %% A 403 for a token without the scope is returned as it is: a new token + %% won't get access, so the session isn't refreshed and the request + %% doesn't run again. + Now = erlang:system_time(second), + Requests = counters:new(1, []), + Config = config_with_callbacks(#{ + oauth_tokens => + {ok, #{ + access_token => <<"global_oauth">>, + refresh_token => <<"refresh_token">>, + expires_at => Now + 3600 + }}, + persist_oauth_tokens => fun(_Scope, _Access, _Refresh, _Expires) -> + error(refreshed) + end + }), + Headers = #{ + <<"www-authenticate">> => <<"Bearer realm=\"hex\", error=\"insufficient_scope\"">> + }, + Response = {ok, {403, Headers, <<"">>}}, + + Fun = fun(Cfg) -> + counters:add(Requests, 1, 1), + <<"Bearer global_oauth">> = maps:get(repo_key, Cfg), + Response + end, + + ?assertEqual( + Response, + hex_cli_auth:with_repo(Config#{repo_organization => <<"acme">>, trusted => true}, Fun) + ), + ?assertEqual(1, counters:get(Requests, 1)), + ok. + +with_repo_rejected_config_repo_key_test(_Config) -> + %% A repo_key in config is the caller's choice for this request, so its + %% rejection is returned instead of the request running with the session. + Now = erlang:system_time(second), + Config = config_with_callbacks(#{ + oauth_tokens => {ok, #{access_token => <<"global_oauth">>, expires_at => Now + 3600}} + }), + + Fun = fun(Cfg) -> + <<"Bearer config_token">> = maps:get(repo_key, Cfg), + invalid_token_response() + end, + + ?assertEqual( + invalid_token_response(), + hex_cli_auth:with_repo( + Config#{ + repo_organization => <<"acme">>, + repo_key => <<"Bearer config_token">>, + trusted => true + }, + Fun + ) + ), + ok. + +with_repo_repo_key_before_oauth_test(_Config) -> + %% A repo_key from get_auth_config that the server accepts is used ahead + %% of the session. + Now = erlang:system_time(second), + Config = config_with_callbacks(#{ + auth_config => #{<<"hexpm:acme">> => #{repo_key => <<"Bearer org_token">>}}, + oauth_tokens => {ok, #{access_token => <<"global_oauth">>, expires_at => Now + 3600}} + }), + + ?assertEqual( + <<"Bearer org_token">>, + hex_cli_auth:with_repo( + Config#{repo_organization => <<"acme">>, trusted => true}, + fun(Cfg) -> maps:get(repo_key, Cfg) end + ) + ), + ok. + +with_repo_replaced_repo_key_test(_Config) -> + %% A repo_key the build tool replaced after the server rejected the old + %% one isn't the rejected credential, so the request runs again with it + %% rather than with the session. + Now = erlang:system_time(second), + Reads = counters:new(1, []), + Config = config_with_callbacks(#{ + oauth_tokens => {ok, #{access_token => <<"global_oauth">>, expires_at => Now + 3600}} + }), + Callbacks = maps:get(cli_auth_callbacks, Config), + GetAuthConfig = fun(<<"hexpm:acme">>) -> + counters:add(Reads, 1, 1), + case counters:get(Reads, 1) of + 1 -> #{repo_key => <<"Bearer old_org_token">>}; + _ -> #{repo_key => <<"Bearer new_org_token">>} + end + end, + + Fun = fun(Cfg) -> + case maps:get(repo_key, Cfg) of + <<"Bearer old_org_token">> -> invalid_token_response(); + RepoKey -> RepoKey + end + end, + + ?assertEqual( + <<"Bearer new_org_token">>, + hex_cli_auth:with_repo( + Config#{ + cli_auth_callbacks => Callbacks#{get_auth_config => GetAuthConfig}, + repo_organization => <<"acme">>, + trusted => true + }, + Fun + ) + ), + ok. + refresh_transport_error_keeps_token_test(_Config) -> %% A refresh that never reached the server says nothing about the stored %% token, so it is kept and the caller is told the difference. @@ -2127,8 +2426,8 @@ with_repo_workload_identity_failed_required_test(_Config) -> ), ok. -with_repo_token_expired_workload_identity_test(_Config) -> - %% A kept token the repository answers token_expired for is exchanged again +with_repo_invalid_token_workload_identity_test(_Config) -> + %% A kept token the repository answers invalid_token for is exchanged again %% even though its expiry has not passed. put_github_oidc_env("https://ci.test/token"), Now = erlang:system_time(second), @@ -2137,7 +2436,7 @@ with_repo_token_expired_workload_identity_test(_Config) -> Fun = fun(Cfg) -> case maps:get(repo_key, Cfg) of - <<"Bearer stale_token">> -> token_expired_response(); + <<"Bearer stale_token">> -> invalid_token_response(); RepoKey -> RepoKey end end, @@ -2145,7 +2444,7 @@ with_repo_token_expired_workload_identity_test(_Config) -> ?assertEqual(<<"Bearer minted.repository:acme">>, hex_cli_auth:with_repo(Config, Fun)), ok. -with_repo_token_expired_workload_identity_renewed_test(_Config) -> +with_repo_invalid_token_workload_identity_renewed_test(_Config) -> %% A token another request already renewed is used instead of exchanging %% again. No fixture answers the CI token URL, so the test crashes if an %% OIDC token is requested. @@ -2162,7 +2461,7 @@ with_repo_token_expired_workload_identity_renewed_test(_Config) -> Fun = fun(Cfg) -> case maps:get(repo_key, Cfg) of - <<"Bearer stale_token">> -> token_expired_response(); + <<"Bearer stale_token">> -> invalid_token_response(); RepoKey -> RepoKey end end, @@ -2170,7 +2469,7 @@ with_repo_token_expired_workload_identity_renewed_test(_Config) -> ?assertEqual(<<"Bearer renewed_token">>, hex_cli_auth:with_repo(Config, Fun)), ok. -with_repo_token_expired_workload_identity_failed_test(_Config) -> +with_repo_invalid_token_workload_identity_failed_test(_Config) -> %% The request needed the token it was renewing, so a refused renewal is %% returned instead of the 401 that asked for it. put_github_oidc_env("https://ci.test/token"), @@ -2181,7 +2480,7 @@ with_repo_token_expired_workload_identity_failed_test(_Config) -> Fun = fun(Cfg) -> <<"Bearer stale_token">> = maps:get(repo_key, Cfg), - token_expired_response() + invalid_token_response() end, ?assertEqual( @@ -2192,6 +2491,36 @@ with_repo_token_expired_workload_identity_failed_test(_Config) -> ), ok. +with_repo_rejected_repo_key_workload_identity_test(_Config) -> + %% Without a session, a rejected repo_key from get_auth_config falls back + %% to the CI job's workload identity. The exchange resolves again under the + %% repo lock, which skips the rejected repo_key too. + put_github_oidc_env("https://ci.test/token"), + Config = workload_identity_config(self(), error), + Callbacks = maps:get(cli_auth_callbacks, Config), + GetAuthConfig = fun(_RepoName) -> #{repo_key => <<"Bearer org_token">>} end, + + Fun = fun(Cfg) -> + case maps:get(repo_key, Cfg) of + <<"Bearer org_token">> -> invalid_token_response(); + RepoKey -> RepoKey + end + end, + + ?assertEqual( + <<"Bearer minted.repository:acme">>, + hex_cli_auth:with_repo( + Config#{cli_auth_callbacks => Callbacks#{get_auth_config => GetAuthConfig}}, Fun + ) + ), + + receive + {workload_identity_persisted, <<"hexpm:acme">>, {ok, _Token}} -> ok + after 100 -> + error(token_not_persisted) + end, + ok. + %%==================================================================== %% Helper Functions %%==================================================================== @@ -2478,9 +2807,10 @@ queue_jwt_bearer_response(Overrides) -> ok. %% @private -%% The 401 hexpm answers a request whose token it considers expired. -token_expired_response() -> - Headers = #{<<"www-authenticate">> => <<"Bearer realm=\"hex\", error=\"token_expired\"">>}, +%% The 401 Hex.pm answers a request whose access token is expired, revoked or +%% doesn't verify. +invalid_token_response() -> + Headers = #{<<"www-authenticate">> => <<"Bearer realm=\"hex\", error=\"invalid_token\"">>}, {ok, {401, Headers, <<"">>}}. %% @private diff --git a/test/hex_http_SUITE.erl b/test/hex_http_SUITE.erl new file mode 100644 index 0000000..74b5560 --- /dev/null +++ b/test/hex_http_SUITE.erl @@ -0,0 +1,121 @@ +-module(hex_http_SUITE). + +-compile([export_all]). + +-include_lib("eunit/include/eunit.hrl"). +-include_lib("common_test/include/ct.hrl"). + +all() -> + [ + bearer_error_test, + bearer_error_parameters_test, + bearer_error_several_challenges_test, + bearer_error_quoted_string_test, + bearer_error_none_test + ]. + +bearer_error_test(_Config) -> + %% What hexpm and repo.hex.pm send + ?assertEqual( + <<"invalid_token">>, + bearer_error(<<"Bearer realm=\"hex\", error=\"invalid_token\"">>) + ), + ?assertEqual( + <<"insufficient_scope">>, + bearer_error(<<"Bearer realm=\"hex\", error=\"insufficient_scope\"">>) + ), + ?assertEqual( + <<"invalid_token">>, + bearer_error( + <<"Bearer realm=\"hex\", error=\"invalid_token\", ", + "error_description=\"The access token expired\"">> + ) + ), + ok. + +bearer_error_parameters_test(_Config) -> + %% Scheme and parameter names are case-insensitive, parameters come in + %% any order, a value can be a token, there can be whitespace around "=" + %% and realm is optional (RFC 9110 section 11.2). + ?assertEqual( + <<"invalid_token">>, + bearer_error(<<"bearer ERROR=\"invalid_token\", Realm=\"hex\"">>) + ), + ?assertEqual(<<"invalid_token">>, bearer_error(<<"Bearer error = invalid_token">>)), + ?assertEqual(<<"invalid_token">>, bearer_error(<<"Bearer error=\"invalid_token\"">>)), + ?assertEqual( + <<"invalid_token">>, bearer_error(<<"Bearer realm=\"hex\",\terror=\"invalid_token\"">>) + ), + ok. + +bearer_error_several_challenges_test(_Config) -> + %% A field can hold several challenges, and a challenge can be a + %% token68 (RFC 9110 section 11.6.1). + ?assertEqual( + <<"invalid_token">>, + bearer_error(<<"Basic realm=\"hex\", Bearer realm=\"hex\", error=\"invalid_token\"">>) + ), + ?assertEqual( + <<"invalid_token">>, + bearer_error(<<"Negotiate a87421000492aa874209af8bc028==, Bearer error=\"invalid_token\"">>) + ), + ?assertEqual( + <<"invalid_token">>, + bearer_error(<<", , Bearer error=\"invalid_token\",, realm=\"hex\"">>) + ), + ?assertEqual(<<"invalid_token">>, bearer_error(<<"Bearer , error=invalid_token">>)), + ?assertEqual( + <<"invalid_token">>, + bearer_error(<<"Basic , realm=\"hex\", Bearer error=invalid_token">>) + ), + %% The first Bearer challenge with an error is the one that counts, so one + %% for another realm without an error doesn't hide it. + ?assertEqual( + <<"invalid_token">>, + bearer_error(<<"Bearer realm=\"first\", Bearer realm=\"hex\", error=invalid_token">>) + ), + ?assertEqual( + undefined, + bearer_error( + <<"Newauth realm=\"apps\", type=1, title=\"Login to \\\"apps\\\"\", ", + "Basic realm=\"simple\"">> + ) + ), + ?assertEqual( + <<"invalid_token">>, + bearer_error( + <<"Newauth realm=\"apps\", type=1, title=\"Login to \\\"apps\\\"\", ", + "Bearer realm=\"hex\", error=\"invalid_token\"">> + ) + ), + ok. + +bearer_error_quoted_string_test(_Config) -> + %% A quoted string can hold commas, escaped quotes and things that look + %% like parameters. + ?assertEqual( + <<"invalid_token">>, + bearer_error( + <<"Bearer error_description=\"say \\\"hi\\\", error=\\\"nope\\\"\", ", + "error=\"invalid_token\"">> + ) + ), + ?assertEqual(<<"a\"b">>, bearer_error(<<"Bearer error=\"a\\\"b\"">>)), + ok. + +bearer_error_none_test(_Config) -> + ?assertEqual(undefined, hex_http:bearer_error(#{})), + ?assertEqual(undefined, bearer_error(<<"Bearer realm=\"hex\"">>)), + ?assertEqual(undefined, bearer_error(<<"Basic realm=\"hex\"">>)), + ?assertEqual(undefined, bearer_error(<<"Bearer">>)), + ?assertEqual(undefined, bearer_error(<<"">>)), + ?assertEqual(undefined, bearer_error(<<"Bearer error=\"unterminated">>)), + %% A parameter read before the field turned malformed is kept + ?assertEqual( + <<"invalid_token">>, + bearer_error(<<"Bearer error=\"invalid_token\", error_description=\"unterminated">>) + ), + ok. + +bearer_error(Value) -> + hex_http:bearer_error(#{<<"www-authenticate">> => Value}). diff --git a/test/support/hex_http_test.erl b/test/support/hex_http_test.erl index 2fdaae5..1f471d0 100644 --- a/test/support/hex_http_test.erl +++ b/test/support/hex_http_test.erl @@ -374,6 +374,20 @@ fixture(post, <>, _, {_, Body}) -> %% OAuth API +%% hexpm authenticates an API request before routing it, so a device or token +%% request that carries a credential it refuses gets a 401 and never reaches +%% the grant. Every credential in these tests is one it would refuse. +fixture(Method, <>, #{<<"authorization">> := _}, _) when + (Method =:= post andalso + (Path =:= <<"/oauth/device_authorization">> orelse Path =:= <<"/oauth/token">>)) orelse + (Method =:= get andalso Path =:= <<"/oidc/audience">>) +-> + Headers = maps:put( + <<"www-authenticate">>, <<"Bearer realm=\"hex\", error=\"invalid_token\"">>, api_headers() + ), + Payload = #{<<"message">> => <<"invalid or expired access token">>}, + {ok, {401, Headers, term_to_binary(Payload)}}; + fixture(post, <>, _, {_, Body}) -> DecodedBody = binary_to_term(Body), #{<<"client_id">> := _ClientId, <<"scope">> := _Scope} = DecodedBody,