mirror of
https://github.com/coder/coder.git
synced 2026-09-01 14:53:15 +08:00
fix(coderd): set Cache-Control: no-store on OAuth2 responses (#28143)
No response from the `/oauth2` route tree set `Cache-Control` at all, so
an intermediary cache or customer-operated reverse proxy was free to
apply a heuristic freshness lifetime to a response carrying a live
credential. RFC 6749 §5.1 and OAuth 2.1 §3.2.3 both make an affirmative
`no-store` directive a MUST for the authorization server.
Adds `httpmw.NoStore`, mounted on the `/oauth2` and
`/api/v2/oauth2-provider` trees, setting `Cache-Control: no-store` and
`Pragma: no-cache` on every response from them. OAuth 2.1 drops `Pragma`
because RFC 9111 §5.4 deprecates it as a request-only field, so sending
both is conformant under either reading. Not operator-configurable,
since both specs say MUST.
## Scope
- **Both trees, not just `POST /oauth2/tokens`.** The mount is one line
either way, and the wider scope also covers DCR registration, client
configuration read and update, the authorize 302 whose `Location` query
carries the code, and `POST /oauth2-provider/apps/{app}/secrets`, which
returns a plaintext client secret. A route added later inherits the
headers, which matters for PLAT-449.
- **A middleware, not a hook in `httpapi.Write`.** Three write paths
never call it: `POST /oauth2/revoke` and `DELETE
/oauth2/clients/{client_id}` write a bare status, and
`writeOAuth2RegistrationError` encodes its own JSON.
- **`/.well-known/*` deliberately excluded.** Public discovery metadata,
and RFC 9728 §5 asks for the opposite treatment. Assertions pin the
exclusion so a later hoist onto a higher router fails CI.
- **Session-credential routes left alone.** `/users/login`,
`/users/otp/change-password`, and `/users/{user}/keys/*` have the same
gap, but PLAT-448 is scoped to OAuth2 and reaching into session auth
changes the risk profile.
Every credential-returning route here is a `POST`, and RFC 9111 §3 bars
heuristic caching of `POST` responses, so this is defense-in-depth
against a non-conformant intermediary rather than a live caching bug.
Both specs say MUST regardless of what caches would actually do.
## Note for PLAT-498
`DELETE /oauth2/tokens` now carries `no-store` and is wrapped in
`apiKeyMiddleware`, which is mounted inside the `/oauth2` tree and
therefore runs after this middleware. It is the one route where both can
write `Cache-Control`, and PLAT-498's write must not replace `no-store`
with something weaker such as `private`. `POST /oauth2/tokens` cannot
overlap, since it deliberately has no `apiKeyMiddleware`.
## Two assumptions testing corrected
- `GET /oauth2/does-not-exist` returns **200**, not 404. Chi runs the
subrouter's middleware chain for unmatched paths, so both headers are
present, but the request falls through to the root router's SPA handler.
The test asserts the headers and deliberately not the status.
- The experiment-disabled case is unreachable from a test binary, since
`RequireExperimentWithDevBypass` short-circuits on `buildinfo.IsDev()`.
A unit test covers the consequence against the `RequireExperiment` it
delegates to.
No schema, `codersdk`, or serpent option changes, so `make gen` produces
no diff. Rollback is a revert.
Refs PLAT-448
This commit is contained in:
@@ -1243,6 +1243,11 @@ func New(options *Options) *API {
|
||||
r.Route("/oauth2", func(r chi.Router) {
|
||||
r.Use(
|
||||
httpmw.RequireExperimentWithDevBypass(api.Experiments, codersdk.ExperimentOAuth2),
|
||||
// Every response from this tree may carry a credential, so none of
|
||||
// them may be retained by an intermediary cache. Mounted after
|
||||
// the gate, so a request the gate rejects gets no headers. That
|
||||
// rejection carries no credential, so it needs none.
|
||||
httpmw.NoStore,
|
||||
)
|
||||
r.Route("/authorize", func(r chi.Router) {
|
||||
r.Use(
|
||||
@@ -2108,6 +2113,10 @@ func New(options *Options) *API {
|
||||
r.Use(
|
||||
apiKeyMiddleware,
|
||||
httpmw.RequireExperimentWithDevBypass(api.Experiments, codersdk.ExperimentOAuth2),
|
||||
// POST /apps/{app}/secrets returns a plaintext client secret,
|
||||
// so this tree falls under the same RFC 6749 §5.1 requirement
|
||||
// as /oauth2.
|
||||
httpmw.NoStore,
|
||||
)
|
||||
r.Route("/apps", func(r chi.Router) {
|
||||
r.Get("/", api.oAuth2ProviderApps())
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
package httpmw
|
||||
|
||||
import "net/http"
|
||||
|
||||
// NoStore sets the response caching headers that OAuth2 requires on any
|
||||
// response that may contain a credential. RFC 6749 §5.1 makes both headers a
|
||||
// MUST for the authorization server; OAuth 2.1 §3.2.3 keeps only no-store,
|
||||
// because RFC 9111 §5.4 deprecates Pragma as a request-only field. Both are
|
||||
// sent so that a client or auditor reading either specification sees a
|
||||
// conformant response.
|
||||
//
|
||||
// The headers are set before the wrapped handler runs, so a handler that
|
||||
// writes its own Cache-Control would win. None does today; the integration
|
||||
// tests pin that across the /oauth2 tree and spot-check
|
||||
// /api/v2/oauth2-provider. Pragma is written unconditionally, so such a
|
||||
// handler's Cache-Control ships alongside Pragma: no-cache.
|
||||
//
|
||||
// chi's middleware.NoCache is not used, though that package is already
|
||||
// imported at the mount site. It strips the ETag-family headers from the
|
||||
// request, which an authorization server has no business doing, and it sends
|
||||
// directives neither specification asks for, where OAuth 2.1 narrows the
|
||||
// requirement rather than widening it.
|
||||
func NoStore(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
|
||||
rw.Header().Set("Cache-Control", "no-store")
|
||||
rw.Header().Set("Pragma", "no-cache")
|
||||
next.ServeHTTP(rw, r)
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,102 @@
|
||||
package httpmw_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/httpmw"
|
||||
)
|
||||
|
||||
func TestNoStore(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
Name string
|
||||
Handler http.HandlerFunc
|
||||
|
||||
expectStatus int
|
||||
expectCacheControl string
|
||||
expectPragma string
|
||||
assert func(t *testing.T, res *httptest.ResponseRecorder)
|
||||
}{
|
||||
{
|
||||
// The POST /oauth2/tokens shape.
|
||||
Name: "OK",
|
||||
Handler: func(rw http.ResponseWriter, _ *http.Request) {
|
||||
rw.WriteHeader(http.StatusOK)
|
||||
_, _ = rw.Write([]byte(`{"access_token":"secret"}`))
|
||||
},
|
||||
expectStatus: http.StatusOK,
|
||||
expectCacheControl: "no-store",
|
||||
expectPragma: "no-cache",
|
||||
},
|
||||
{
|
||||
// The DELETE /oauth2/clients/{client_id} shape: headers on a
|
||||
// response with no body.
|
||||
Name: "NoContent",
|
||||
Handler: func(rw http.ResponseWriter, _ *http.Request) {
|
||||
rw.WriteHeader(http.StatusNoContent)
|
||||
},
|
||||
expectStatus: http.StatusNoContent,
|
||||
expectCacheControl: "no-store",
|
||||
expectPragma: "no-cache",
|
||||
},
|
||||
{
|
||||
// The POST /oauth2/authorize shape: http.Redirect writes its own
|
||||
// headers without clearing the map.
|
||||
Name: "Redirect",
|
||||
Handler: func(rw http.ResponseWriter, r *http.Request) {
|
||||
http.Redirect(rw, r, "https://example.com/callback?code=abc", http.StatusFound)
|
||||
},
|
||||
expectStatus: http.StatusFound,
|
||||
expectCacheControl: "no-store",
|
||||
expectPragma: "no-cache",
|
||||
assert: func(t *testing.T, res *httptest.ResponseRecorder) {
|
||||
require.Equal(t, "https://example.com/callback?code=abc", res.Header().Get("Location"))
|
||||
},
|
||||
},
|
||||
{
|
||||
// The revoke.go and registration.go shape: a bare WriteHeader,
|
||||
// never httpapi.Write.
|
||||
Name: "BareWriteHeaderError",
|
||||
Handler: func(rw http.ResponseWriter, _ *http.Request) {
|
||||
rw.WriteHeader(http.StatusBadRequest)
|
||||
},
|
||||
expectStatus: http.StatusBadRequest,
|
||||
expectCacheControl: "no-store",
|
||||
expectPragma: "no-cache",
|
||||
},
|
||||
{
|
||||
// The headers are advisory: a handler that writes its own
|
||||
// Cache-Control wins, and Pragma survives alongside it.
|
||||
Name: "HandlerOverwrites",
|
||||
Handler: func(rw http.ResponseWriter, _ *http.Request) {
|
||||
rw.Header().Set("Cache-Control", "private")
|
||||
rw.WriteHeader(http.StatusOK)
|
||||
},
|
||||
expectStatus: http.StatusOK,
|
||||
expectCacheControl: "private",
|
||||
expectPragma: "no-cache",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.Name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
res := httptest.NewRecorder()
|
||||
httpmw.NoStore(tt.Handler).ServeHTTP(res, req)
|
||||
|
||||
require.Equal(t, tt.expectStatus, res.Code)
|
||||
require.Equal(t, tt.expectCacheControl, res.Header().Get("Cache-Control"))
|
||||
require.Equal(t, tt.expectPragma, res.Header().Get("Pragma"))
|
||||
if tt.assert != nil {
|
||||
tt.assert(t, res)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,420 @@
|
||||
package oauth2provider_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/coderdtest"
|
||||
"github.com/coder/coder/v2/coderd/oauth2provider/oauth2providertest"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
"github.com/coder/coder/v2/testutil"
|
||||
)
|
||||
|
||||
// TestOAuth2NoStoreHeaders asserts Cache-Control: no-store and Pragma:
|
||||
// no-cache on every response from the /oauth2 tree, credential-bearing or not.
|
||||
// Three write paths never call httpapi.Write, so these are what prove the
|
||||
// middleware reaches them: POST /oauth2/revoke and DELETE
|
||||
// /oauth2/clients/{client_id} write a bare status, and
|
||||
// writeOAuth2RegistrationError encodes its own JSON.
|
||||
//
|
||||
// The cases at the end pin the exclusion of the /.well-known/* metadata
|
||||
// endpoints, failing if the middleware is hoisted onto a higher route tree.
|
||||
func TestOAuth2NoStoreHeaders(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
oauth2providertest.EnableDCR(t, client)
|
||||
baseURL := client.URL.String()
|
||||
|
||||
t.Run("TokenExchange", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
app, secret := oauth2providertest.CreateTestOAuth2App(t, client)
|
||||
verifier, challenge := oauth2providertest.GeneratePKCE(t)
|
||||
code := authorizationCode(t, client, baseURL, app.ID.String(), challenge)
|
||||
|
||||
form := url.Values{}
|
||||
form.Set("grant_type", "authorization_code")
|
||||
form.Set("code", code)
|
||||
form.Set("client_id", app.ID.String())
|
||||
form.Set("client_secret", secret)
|
||||
form.Set("code_verifier", verifier)
|
||||
form.Set("redirect_uri", oauth2providertest.TestRedirectURI)
|
||||
|
||||
resp := doRequest(ctx, t, http.MethodPost, baseURL+"/oauth2/tokens", strings.NewReader(form.Encode()), formContentType)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
})
|
||||
|
||||
t.Run("TokenExchangeInvalidClient", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
app, _ := oauth2providertest.CreateTestOAuth2App(t, client)
|
||||
verifier, challenge := oauth2providertest.GeneratePKCE(t)
|
||||
code := authorizationCode(t, client, baseURL, app.ID.String(), challenge)
|
||||
|
||||
form := url.Values{}
|
||||
form.Set("grant_type", "authorization_code")
|
||||
form.Set("code", code)
|
||||
form.Set("client_id", app.ID.String())
|
||||
form.Set("client_secret", "not-the-client-secret")
|
||||
form.Set("code_verifier", verifier)
|
||||
form.Set("redirect_uri", oauth2providertest.TestRedirectURI)
|
||||
|
||||
resp := doRequest(ctx, t, http.MethodPost, baseURL+"/oauth2/tokens", strings.NewReader(form.Encode()), formContentType)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusUnauthorized, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
// Both writers coexist: WriteOAuth2Error sets this one after the
|
||||
// middleware has set its own.
|
||||
require.Equal(t, `Basic realm="coder"`, resp.Header.Get("WWW-Authenticate"))
|
||||
})
|
||||
|
||||
t.Run("AuthorizeRedirect", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
app, _ := oauth2providertest.CreateTestOAuth2App(t, client)
|
||||
_, challenge := oauth2providertest.GeneratePKCE(t)
|
||||
|
||||
resp := doRequest(ctx, t, http.MethodPost, authorizeURL(baseURL, app.ID.String(), challenge), nil, sessionToken(client))
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusFound, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
|
||||
// The credential this response carries is in the Location query.
|
||||
location, err := url.Parse(resp.Header.Get("Location"))
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, location.Query().Get("code"))
|
||||
})
|
||||
|
||||
t.Run("AuthorizeConsentPage", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
app, _ := oauth2providertest.CreateTestOAuth2App(t, client)
|
||||
_, challenge := oauth2providertest.GeneratePKCE(t)
|
||||
|
||||
resp := doRequest(ctx, t, http.MethodGet, authorizeURL(baseURL, app.ID.String(), challenge), nil, sessionToken(client))
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
})
|
||||
|
||||
t.Run("AuthorizeStaticErrorPage", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
app, _ := oauth2providertest.CreateTestOAuth2App(t, client)
|
||||
_, challenge := oauth2providertest.GeneratePKCE(t)
|
||||
|
||||
// An unsupported response_type renders a static error page rather
|
||||
// than going through httpapi.
|
||||
uri := strings.Replace(authorizeURL(baseURL, app.ID.String(), challenge), "response_type=code", "response_type=token", 1)
|
||||
resp := doRequest(ctx, t, http.MethodGet, uri, nil, sessionToken(client))
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
})
|
||||
|
||||
t.Run("Revoke", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
app, secret := oauth2providertest.CreateTestOAuth2App(t, client)
|
||||
verifier, challenge := oauth2providertest.GeneratePKCE(t)
|
||||
code := authorizationCode(t, client, baseURL, app.ID.String(), challenge)
|
||||
token := oauth2providertest.ExchangeCodeForToken(t, baseURL, oauth2providertest.TokenExchangeParams{
|
||||
GrantType: "authorization_code",
|
||||
Code: code,
|
||||
ClientID: app.ID.String(),
|
||||
ClientSecret: secret,
|
||||
CodeVerifier: verifier,
|
||||
RedirectURI: oauth2providertest.TestRedirectURI,
|
||||
})
|
||||
|
||||
form := url.Values{}
|
||||
form.Set("token", token.RefreshToken)
|
||||
form.Set("client_id", app.ID.String())
|
||||
|
||||
// RFC 7009 success is a bare WriteHeader(200), never httpapi.Write.
|
||||
resp := doRequest(ctx, t, http.MethodPost, baseURL+"/oauth2/revoke", strings.NewReader(form.Encode()), formContentType)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
})
|
||||
|
||||
t.Run("DeleteTokens", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
app, _ := oauth2providertest.CreateTestOAuth2App(t, client)
|
||||
|
||||
uri := fmt.Sprintf("%s/oauth2/tokens?client_id=%s", baseURL, app.ID.String())
|
||||
resp := doRequest(ctx, t, http.MethodDelete, uri, nil, sessionToken(client))
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusNoContent, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
})
|
||||
|
||||
t.Run("Register", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
body := registrationBody(t, codersdk.OAuth2ClientRegistrationRequest{
|
||||
RedirectURIs: []string{"https://example.com/callback"},
|
||||
ClientName: fmt.Sprintf("nostore-register-%s", testutil.MustRandString(t, 10)),
|
||||
})
|
||||
|
||||
resp := doRequest(ctx, t, http.MethodPost, baseURL+"/oauth2/register", strings.NewReader(body), jsonContentType)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusCreated, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
})
|
||||
|
||||
t.Run("RegisterInvalidMetadata", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
body := registrationBody(t, codersdk.OAuth2ClientRegistrationRequest{
|
||||
RedirectURIs: []string{"not-a-url"},
|
||||
})
|
||||
|
||||
// Rejected by writeOAuth2RegistrationError, which encodes its own
|
||||
// JSON rather than calling httpapi.Write.
|
||||
resp := doRequest(ctx, t, http.MethodPost, baseURL+"/oauth2/register", strings.NewReader(body), jsonContentType)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
})
|
||||
|
||||
t.Run("GetClientConfiguration", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
registration := registerClient(ctx, t, client)
|
||||
|
||||
uri := fmt.Sprintf("%s/oauth2/clients/%s", baseURL, registration.ClientID)
|
||||
resp := doRequest(ctx, t, http.MethodGet, uri, nil, bearer(registration.RegistrationAccessToken))
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
})
|
||||
|
||||
t.Run("PutClientConfiguration", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
registration := registerClient(ctx, t, client)
|
||||
body := registrationBody(t, codersdk.OAuth2ClientRegistrationRequest{
|
||||
RedirectURIs: []string{"https://example.com/updated-callback"},
|
||||
ClientName: fmt.Sprintf("nostore-updated-%s", testutil.MustRandString(t, 10)),
|
||||
})
|
||||
|
||||
uri := fmt.Sprintf("%s/oauth2/clients/%s", baseURL, registration.ClientID)
|
||||
resp := doRequest(ctx, t, http.MethodPut, uri, strings.NewReader(body), jsonContentType, bearer(registration.RegistrationAccessToken))
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
})
|
||||
|
||||
t.Run("DeleteClientConfiguration", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
registration := registerClient(ctx, t, client)
|
||||
|
||||
// RFC 7592 §2.3's own example shows no-store on exactly this 204.
|
||||
uri := fmt.Sprintf("%s/oauth2/clients/%s", baseURL, registration.ClientID)
|
||||
resp := doRequest(ctx, t, http.MethodDelete, uri, nil, bearer(registration.RegistrationAccessToken))
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusNoContent, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
})
|
||||
|
||||
t.Run("UnmatchedPath", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
// Chi runs a subrouter's middleware chain around its unmatched-path
|
||||
// handling, so a path with no route still carries the headers. The
|
||||
// status is not asserted: the request falls through to the root
|
||||
// router's SPA handler, which is not this middleware's business.
|
||||
resp := doRequest(ctx, t, http.MethodGet, baseURL+"/oauth2/does-not-exist", nil)
|
||||
defer resp.Body.Close()
|
||||
requireNoStore(t, resp)
|
||||
})
|
||||
|
||||
// Discovery metadata is public and RFC 9728 §5 asks for it to be
|
||||
// cacheable. These do not prove it is, since the endpoints advertise no
|
||||
// freshness lifetime at all. What they pin is that no blanket middleware
|
||||
// has landed on a router reaching them: this one would stamp no-store,
|
||||
// and an "authenticated responses are not shared-cacheable" middleware
|
||||
// would stamp private or max-age=0. An explicit freshness lifetime added
|
||||
// here later, which RFC 9728 §5 encourages, still passes.
|
||||
for _, path := range []string{
|
||||
"/.well-known/oauth-authorization-server",
|
||||
"/.well-known/oauth-protected-resource",
|
||||
} {
|
||||
t.Run("Cacheable"+path, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
resp := doRequest(ctx, t, http.MethodGet, baseURL+path, nil)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
for _, directive := range []string{"no-store", "private", "no-cache", "max-age=0"} {
|
||||
require.NotContains(t, resp.Header.Get("Cache-Control"), directive)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestOAuth2ProviderNoStoreHeaders asserts the same headers on the
|
||||
// /api/v2/oauth2-provider tree, where POST /apps/{app}/secrets returns
|
||||
// ClientSecretFull in plaintext and so meets RFC 6749 §5.1's predicate as
|
||||
// squarely as anything under /oauth2.
|
||||
func TestOAuth2ProviderNoStoreHeaders(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
client := coderdtest.New(t, nil)
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
baseURL := client.URL.String()
|
||||
|
||||
t.Run("CreateAppSecret", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
app, _ := oauth2providertest.CreateTestOAuth2App(t, client)
|
||||
|
||||
uri := fmt.Sprintf("%s/api/v2/oauth2-provider/apps/%s/secrets", baseURL, app.ID)
|
||||
resp := doRequest(ctx, t, http.MethodPost, uri, nil, sessionToken(client))
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusCreated, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
|
||||
var secret codersdk.OAuth2ProviderAppSecretFull
|
||||
require.NoError(t, json.NewDecoder(resp.Body).Decode(&secret))
|
||||
require.NotEmpty(t, secret.ClientSecretFull)
|
||||
})
|
||||
|
||||
t.Run("ListApps", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
// A credential-free route still carries the headers, which shows the
|
||||
// mount is on the tree rather than on one handler.
|
||||
resp := doRequest(ctx, t, http.MethodGet, baseURL+"/api/v2/oauth2-provider/apps", nil, sessionToken(client))
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
requireNoStore(t, resp)
|
||||
})
|
||||
}
|
||||
|
||||
func requireNoStore(t *testing.T, resp *http.Response) {
|
||||
t.Helper()
|
||||
|
||||
require.Equal(t, "no-store", resp.Header.Get("Cache-Control"))
|
||||
require.Equal(t, "no-cache", resp.Header.Get("Pragma"))
|
||||
}
|
||||
|
||||
// doRequest performs a request without following redirects, so a 302's own
|
||||
// headers can be asserted rather than the redirect target's.
|
||||
func doRequest(ctx context.Context, t *testing.T, method, uri string, body io.Reader, opts ...func(*http.Request)) *http.Response {
|
||||
t.Helper()
|
||||
|
||||
req, err := http.NewRequestWithContext(ctx, method, uri, body)
|
||||
require.NoError(t, err)
|
||||
for _, opt := range opts {
|
||||
opt(req)
|
||||
}
|
||||
|
||||
httpClient := &http.Client{
|
||||
CheckRedirect: func(_ *http.Request, _ []*http.Request) error {
|
||||
return http.ErrUseLastResponse
|
||||
},
|
||||
}
|
||||
resp, err := httpClient.Do(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
return resp
|
||||
}
|
||||
|
||||
func formContentType(r *http.Request) {
|
||||
r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
}
|
||||
|
||||
func jsonContentType(r *http.Request) {
|
||||
r.Header.Set("Content-Type", "application/json")
|
||||
}
|
||||
|
||||
func sessionToken(client *codersdk.Client) func(*http.Request) {
|
||||
return func(r *http.Request) {
|
||||
r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken())
|
||||
}
|
||||
}
|
||||
|
||||
func bearer(token string) func(*http.Request) {
|
||||
return func(r *http.Request) {
|
||||
r.Header.Set("Authorization", "Bearer "+token)
|
||||
}
|
||||
}
|
||||
|
||||
func authorizeURL(baseURL, clientID, challenge string) string {
|
||||
query := url.Values{}
|
||||
query.Set("client_id", clientID)
|
||||
query.Set("response_type", "code")
|
||||
query.Set("redirect_uri", oauth2providertest.TestRedirectURI)
|
||||
query.Set("state", "state")
|
||||
query.Set("code_challenge", challenge)
|
||||
query.Set("code_challenge_method", "S256")
|
||||
|
||||
return baseURL + "/oauth2/authorize?" + query.Encode()
|
||||
}
|
||||
|
||||
func authorizationCode(t *testing.T, client *codersdk.Client, baseURL, clientID, challenge string) string {
|
||||
t.Helper()
|
||||
|
||||
state := oauth2providertest.GenerateState(t)
|
||||
return oauth2providertest.AuthorizeOAuth2App(t, client, baseURL, oauth2providertest.AuthorizeParams{
|
||||
ClientID: clientID,
|
||||
ResponseType: "code",
|
||||
RedirectURI: oauth2providertest.TestRedirectURI,
|
||||
State: state,
|
||||
CodeChallenge: challenge,
|
||||
CodeChallengeMethod: "S256",
|
||||
})
|
||||
}
|
||||
|
||||
func registrationBody(t *testing.T, req codersdk.OAuth2ClientRegistrationRequest) string {
|
||||
t.Helper()
|
||||
|
||||
body, err := json.Marshal(req)
|
||||
require.NoError(t, err)
|
||||
|
||||
return string(body)
|
||||
}
|
||||
|
||||
func registerClient(ctx context.Context, t *testing.T, client *codersdk.Client) codersdk.OAuth2ClientRegistrationResponse {
|
||||
t.Helper()
|
||||
|
||||
registration, err := client.PostOAuth2ClientRegistration(ctx, codersdk.OAuth2ClientRegistrationRequest{
|
||||
RedirectURIs: []string{"https://example.com/callback"},
|
||||
ClientName: fmt.Sprintf("nostore-client-%s", testutil.MustRandString(t, 10)),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
return registration
|
||||
}
|
||||
Reference in New Issue
Block a user