feat: HTTP middleware #214
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| // Package http provides HTTP utilities and handler middleware. | ||
| package http | ||
|
|
||
| import ( | ||
| "net/http" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/clerk/clerk-sdk-go/v2" | ||
| "github.com/clerk/clerk-sdk-go/v2/jwt" | ||
| ) | ||
|
|
||
| // RequireHeaderAuthorization will respond with HTTP 403 Forbidden if | ||
| // the Authorization header does not contain a valid session token. | ||
| func RequireHeaderAuthorization(opts ...AuthorizationOption) func(http.Handler) http.Handler { | ||
| return func(next http.Handler) http.Handler { | ||
| return WithHeaderAuthorization(opts...)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| claims, ok := clerk.SessionClaimsFromContext(r.Context()) | ||
| if !ok || claims == nil { | ||
| w.WriteHeader(http.StatusForbidden) | ||
| return | ||
| } | ||
| next.ServeHTTP(w, r) | ||
| })) | ||
| } | ||
| } | ||
|
|
||
| // WithHeaderAuthorization checks the Authorization request header | ||
| // for a valid Clerk authorization JWT. The token is parsed and verified | ||
| // and the active session claims are written to the http.Request context. | ||
| // The middleware uses Bearer authentication, so the Authorization header | ||
| // is expected to have the following format: | ||
| // Authorization: Bearer <token> | ||
| func WithHeaderAuthorization(opts ...AuthorizationOption) func(http.Handler) http.Handler { | ||
| return func(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| authorization := strings.TrimSpace(r.Header.Get("Authorization")) | ||
| if authorization == "" { | ||
| next.ServeHTTP(w, r) | ||
| return | ||
| } | ||
|
|
||
| token := strings.TrimPrefix(authorization, "Bearer ") | ||
| _, err := jwt.Decode(r.Context(), &jwt.DecodeParams{Token: token}) | ||
| if err != nil { | ||
| next.ServeHTTP(w, r) | ||
| return | ||
| } | ||
|
|
||
| params := &AuthorizationParams{} | ||
| for _, opt := range opts { | ||
| err = opt(params) | ||
| if err != nil { | ||
| w.WriteHeader(http.StatusUnauthorized) | ||
| return | ||
| } | ||
| } | ||
| params.Token = token | ||
agis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| claims, err := jwt.Verify(r.Context(), ¶ms.VerifyParams) | ||
| if err != nil { | ||
| w.WriteHeader(http.StatusUnauthorized) | ||
| return | ||
| } | ||
|
|
||
| // Token was verified. Add the session claims to the request context | ||
| newCtx := clerk.ContextWithSessionClaims(r.Context(), claims) | ||
| next.ServeHTTP(w, r.WithContext(newCtx)) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| type AuthorizationParams struct { | ||
| jwt.VerifyParams | ||
| } | ||
|
|
||
| // AuthorizationOption is a functional parameter for configuring | ||
| // authorization options. | ||
| type AuthorizationOption func(*AuthorizationParams) error | ||
|
|
||
| // AuthorizedParty sets the authorized parties that will be checked | ||
| // against the azp JWT claim. | ||
| func AuthorizedParty(parties ...string) AuthorizationOption { | ||
| return func(params *AuthorizationParams) error { | ||
| params.SetAuthorizedParties(parties...) | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| // CustomClaims allows to pass a type (e.g. struct), which will be populated with the token claims based on json tags. | ||
| // You must pass a pointer for this option to work. | ||
| func CustomClaims(claims any) AuthorizationOption { | ||
|
There was a problem hiding this comment. ℹ️ I think you're already aware but just adding a reference for #202 before we solidify the option design! |
||
| return func(params *AuthorizationParams) error { | ||
| params.CustomClaims = claims | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| // Leeway allows to set a custom leeway when comparing time values | ||
| // for JWT verification. | ||
| // The leeway gives some extra time to the token. That is, if the | ||
| // token is expired, it will still be accepted for 'leeway' amount | ||
| // of time. | ||
| // This option accomodates for clock skew. | ||
| func Leeway(leeway time.Duration) AuthorizationOption { | ||
| return func(params *AuthorizationParams) error { | ||
| params.Leeway = leeway | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| // ProxyURL can be used to set the URL that proxies the Clerk Frontend | ||
| // API. Useful for proxy based setups. | ||
| // See https://clerk.com/docs/advanced-usage/using-proxies | ||
| func ProxyURL(proxyURL string) AuthorizationOption { | ||
|
There was a problem hiding this comment. 💡 Shall we validate that |
||
| return func(params *AuthorizationParams) error { | ||
| params.ProxyURL = clerk.String(proxyURL) | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| // Satellite can be used to signify that the authorization happens | ||
| // on a satellite domain. | ||
| // See https://clerk.com/docs/advanced-usage/satellite-domains | ||
| func Satellite(isSatellite bool) AuthorizationOption { | ||
|
There was a problem hiding this comment. ❓ Is there any reason for someone to call this as There was a problem hiding this comment. I guess there's not. I mainly copied whatever was there before. The only thing I can think of is the usage difference opts := []Opt{}
if isSatellite {
opts = append(opts, Satellite())
}
WithHeaderAuthorization(opts...)
// vs
WithHeaderAuthorization(Satellite(isSatellite))I believe the above is a valid case for leaving the API as is. |
||
| return func(params *AuthorizationParams) error { | ||
| params.IsSatellite = isSatellite | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| // JSONWebKey allows to provide a custom JSON Web Key (JWK) based on | ||
| // which the authorization JWT will be verified. | ||
| // When verifying the authorization JWT without a custom key, the JWK | ||
| // will be fetched from the Clerk API and cached for one hour, then | ||
| // the JWK will be fetched again from the Clerk API. | ||
| // Passing a custom JSON Web Key means that no request to fetch JSON | ||
| // web keys will be made. It's the caller's responsibility to refresh | ||
| // the JWK when keys are rolled. | ||
| func JSONWebKey(key string) AuthorizationOption { | ||
| return func(params *AuthorizationParams) error { | ||
| // From the Clerk docs: "Note that the JWT Verification key is not in | ||
|
There was a problem hiding this comment. 🔧 Might be worth adding a unit test here, even for illustrating the use a bit better. There was a problem hiding this comment. My intention was to add an integration test for the options, but setting the tests up took more time than expected. I'm planning to get back to this once I find some time. |
||
| // PEM format, the header and footer are missing, in order to be shorter | ||
| // and single-line for easier setup." | ||
| if !strings.HasPrefix(key, "-----BEGIN") { | ||
|
There was a problem hiding this comment. I'm wondering whether it's worth also checking for if !strings.HasPrefix(...) {
// add prefix
}
if !strings.HasSuffix(...) {
// add suffix
}There was a problem hiding this comment. Not sure about this in general. If it was my decision, I think we should always require the full key here. I simply carried over the logic from v1, but I could add a check for the suffix as well. There was a problem hiding this comment. It seems to me that it would be more fool-proof than what v1 does, but then again I understand if you decide it's not even worth it, since it's an edge case. Up to you! :) |
||
| key = "-----BEGIN PUBLIC KEY-----\n" + key + "\n-----END PUBLIC KEY-----" | ||
| } | ||
| jwk, err := clerk.JSONWebKeyFromPEM(key) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| params.JWK = jwk | ||
| return nil | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package http | ||
|
|
||
| import ( | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "testing" | ||
|
|
||
| "github.com/clerk/clerk-sdk-go/v2" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestWithHeaderAuthorization_InvalidAuthorization(t *testing.T) { | ||
| ts := httptest.NewServer(WithHeaderAuthorization()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| _, ok := clerk.SessionClaimsFromContext(r.Context()) | ||
| require.False(t, ok) | ||
| _, err := w.Write([]byte("{}")) | ||
| require.NoError(t, err) | ||
| }))) | ||
| defer ts.Close() | ||
|
|
||
| clerk.SetBackend(clerk.NewBackend(&clerk.BackendConfig{ | ||
| HTTPClient: ts.Client(), | ||
| URL: &ts.URL, | ||
| })) | ||
|
|
||
| // Request without Authorization header | ||
| req, err := http.NewRequest(http.MethodGet, ts.URL, nil) | ||
| require.NoError(t, err) | ||
| res, err := ts.Client().Do(req) | ||
| require.NoError(t, err) | ||
| require.Equal(t, http.StatusOK, res.StatusCode) | ||
|
|
||
| // Request with invalid Authorization header | ||
| req.Header.Add("authorization", "Bearer whatever") | ||
| res, err = ts.Client().Do(req) | ||
| require.NoError(t, err) | ||
| require.Equal(t, http.StatusOK, res.StatusCode) | ||
| } | ||
|
|
||
| func TestRequireHeaderAuthorization_InvalidAuthorization(t *testing.T) { | ||
| ts := httptest.NewServer(RequireHeaderAuthorization()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| _, err := w.Write([]byte("{}")) | ||
| require.NoError(t, err) | ||
| }))) | ||
| defer ts.Close() | ||
|
|
||
| clerk.SetBackend(clerk.NewBackend(&clerk.BackendConfig{ | ||
| HTTPClient: ts.Client(), | ||
| URL: &ts.URL, | ||
| })) | ||
|
|
||
| // Request without Authorization header | ||
| req, err := http.NewRequest(http.MethodGet, ts.URL, nil) | ||
| require.NoError(t, err) | ||
| res, err := ts.Client().Do(req) | ||
| require.NoError(t, err) | ||
| require.Equal(t, http.StatusForbidden, res.StatusCode) | ||
|
|
||
| // Request with invalid Authorization header | ||
| req.Header.Add("authorization", "Bearer whatever") | ||
| res, err = ts.Client().Do(req) | ||
| require.NoError(t, err) | ||
| require.Equal(t, http.StatusForbidden, res.StatusCode) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| package clerk | ||
|
|
||
| import ( | ||
| "crypto/x509" | ||
| "encoding/pem" | ||
| "fmt" | ||
|
|
||
| "github.com/go-jose/go-jose/v3" | ||
| ) | ||
|
|
||
| type JSONWebKeySet struct { | ||
| APIResource | ||
| Keys []JSONWebKey `json:"keys"` | ||
| } | ||
|
|
||
| type JSONWebKey struct { | ||
| APIResource | ||
| raw jose.JSONWebKey | ||
| Key any `json:"key"` | ||
| KeyID string `json:"kid"` | ||
| Algorithm string `json:"alg"` | ||
| Use string `json:"use"` | ||
| } | ||
|
|
||
| func (k *JSONWebKey) UnmarshalJSON(data []byte) error { | ||
|
There was a problem hiding this comment. ❓ There's probably good reason, I just don't see it yet, would you mind explaining quickly why the custom deserializer is required? There was a problem hiding this comment. I wanted to avoid using a go-jose type and instead make sure we use only our own package's (clerk) types everywhere. There's no reason to expose types that are meant for internal use anywhere. We unfortunately do this in v1 of our SDK. The go-jose library is able to decode JSON web keys and we can leverage that to get a response from our Backend API GET /v1/jwks endpoint and translate it to our own Unfortunately, it looks like extracting the key ID and key from a JWK response is not that straightforward. Depending on the algorithm that's used the JWK response might have different fields set, each with different meaning. The go-jose library handles unmarshaling nicely, but its raw unmarshaled type is unexported. So, what I thought we could do is use the jose.JSONWebKey type to unmarshal the raw JSON and then copy over the resulting fields to our type. I initially tried type aliasing but it didn't work as expected. Hope the above makes the decision clearer. Let me know if I should offer more details! Perhaps I can add a comment explaining what's going on here? There was a problem hiding this comment.
Very good call. This provides us with flexibility to change the underlying library (go-jose) without it being a breaking change. There was a problem hiding this comment. I see, yes good call indeed. Now that the field is unexported the purpose is also much clearer! |
||
| err := k.raw.UnmarshalJSON(data) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| k.Key = k.raw.Key | ||
| k.KeyID = k.raw.KeyID | ||
| k.Algorithm = k.raw.Algorithm | ||
| k.Use = k.raw.Use | ||
| return nil | ||
| } | ||
|
|
||
| // JSONWebKeyFromPEM returns a JWK from an RSA key. | ||
| func JSONWebKeyFromPEM(key string) (*JSONWebKey, error) { | ||
| block, _ := pem.Decode([]byte(key)) | ||
| if block == nil { | ||
| return nil, fmt.Errorf("invalid PEM-encoded block") | ||
| } | ||
|
|
||
| if block.Type != "PUBLIC KEY" { | ||
| return nil, fmt.Errorf("invalid key type, expected a public key") | ||
| } | ||
|
|
||
| rsaPublicKey, err := x509.ParsePKIXPublicKey(block.Bytes) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to parse public key: %w", err) | ||
| } | ||
|
|
||
| return &JSONWebKey{ | ||
| Key: rsaPublicKey, | ||
| Algorithm: "RS256", | ||
| }, nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| package jwks | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "net/http" | ||
| "testing" | ||
|
|
||
| "github.com/clerk/clerk-sdk-go/v2" | ||
| "github.com/clerk/clerk-sdk-go/v2/clerktest" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestJWKGet(t *testing.T) { | ||
| key := map[string]any{ | ||
| "use": "sig", | ||
| "kty": "RSA", | ||
| "kid": "the-kid", | ||
| "alg": "RS256", | ||
| "n": "the-key", | ||
| "e": "AQAB", | ||
| } | ||
| out := map[string]any{ | ||
| "keys": []map[string]any{key}, | ||
| } | ||
| raw, err := json.Marshal(out) | ||
| require.NoError(t, err) | ||
|
|
||
| clerk.SetBackend(clerk.NewBackend(&clerk.BackendConfig{ | ||
| HTTPClient: &http.Client{ | ||
| Transport: &clerktest.RoundTripper{ | ||
| T: t, | ||
| Out: raw, | ||
| Path: "/v1/jwks", | ||
| Method: http.MethodGet, | ||
| }, | ||
| }, | ||
| })) | ||
| jwk, err := Get(context.Background(), &GetParams{}) | ||
| require.NoError(t, err) | ||
| require.Equal(t, 1, len(jwk.Keys)) | ||
| require.NotNil(t, jwk.Keys[0].Key) | ||
| require.Equal(t, key["use"], jwk.Keys[0].Use) | ||
| require.Equal(t, key["alg"], jwk.Keys[0].Algorithm) | ||
| require.Equal(t, key["kid"], jwk.Keys[0].KeyID) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| // Package jwks provides access to the JWKS endpoint. | ||
| package jwks | ||
|
|
||
| import ( | ||
| "context" | ||
| "net/http" | ||
|
|
||
| "github.com/clerk/clerk-sdk-go/v2" | ||
| ) | ||
|
|
||
| const path = "/jwks" | ||
|
|
||
| type GetParams struct { | ||
| clerk.APIParams | ||
| } | ||
|
|
||
| // Get retrieves a JSON Web Key set. | ||
| func Get(ctx context.Context, params *GetParams) (*clerk.JSONWebKeySet, error) { | ||
| req := clerk.NewAPIRequest(http.MethodGet, path) | ||
| req.SetParams(params) | ||
|
|
||
| set := &clerk.JSONWebKeySet{} | ||
| err := clerk.GetBackend().Call(ctx, req, set) | ||
| return set, err | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Conceptually speaking, what's the difference between
r.Context()andctxhere?Actually, why does
WithHeaderAuthorizationaccepts a context in the first place? Shouldn't it work off of the incoming request's context?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, I had the same doubts.
I left the
ctxcontext for possible future usage.Although it's not actually needed, if we don't add it now and we need it in a future version, we'll break the API.
Do you think it's better to remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I can think of a good usage pattern here too, since the middleware setup doesn't happen within an operation context. Perhaps we should remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the context from the arguments.