From 9c9e7106622c32fe7cc8758f7abc912d366d0b30 Mon Sep 17 00:00:00 2001 From: Sam Thanawalla Date: Mon, 22 Sep 2025 17:05:11 +0000 Subject: [PATCH 1/5] internal/oauthex: add dynamic client registration This CL implements https://www.rfc-editor.org/rfc/rfc7591.html For #493 --- internal/oauthex/auth_meta.go | 167 +++++++++++++++++- internal/oauthex/auth_meta_test.go | 131 +++++++++++++- .../oauthex/testdata/client-auth-meta.json | 9 + 3 files changed, 301 insertions(+), 6 deletions(-) create mode 100644 internal/oauthex/testdata/client-auth-meta.json diff --git a/internal/oauthex/auth_meta.go b/internal/oauthex/auth_meta.go index 1f075f8a..f91ac026 100644 --- a/internal/oauthex/auth_meta.go +++ b/internal/oauthex/auth_meta.go @@ -8,20 +8,23 @@ package oauthex import ( + "bytes" "context" + "encoding/json" "errors" "fmt" + "io" "net/http" ) -// AuthServerMeta represents the metadata for an OAuth 2.0 authorization server, +// AuthServerMetadata represents the metadata for an OAuth 2.0 authorization server, // as defined in [RFC 8414]. // // Not supported: // - signed metadata // // [RFC 8414]: https://tools.ietf.org/html/rfc8414) -type AuthServerMeta struct { +type AuthServerMetadata struct { // GENERATED BY GEMINI 2.5. // Issuer is the REQUIRED URL identifying the authorization server. @@ -109,6 +112,106 @@ type AuthServerMeta struct { CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported,omitempty"` } +// AuthClientMetadata represents the client metadata fields for the DCR POST request (RFC 7591). +type AuthClientMetadata struct { + // RedirectURIs is a REQUIRED JSON array of redirection URI strings for use in + // redirect-based flows (such as the authorization code grant). + RedirectURIs []string `json:"redirect_uris"` + + // TokenEndpointAuthMethod is an OPTIONAL string indicator of the requested + // authentication method for the token endpoint. + // If omitted, the default is "client_secret_basic". + TokenEndpointAuthMethod string `json:"token_endpoint_auth_method,omitempty"` + + // GrantTypes is an OPTIONAL JSON array of OAuth 2.0 grant type strings + // that the client will restrict itself to using. + // If omitted, the default is ["authorization_code"]. + GrantTypes []string `json:"grant_types,omitempty"` + + // ResponseTypes is an OPTIONAL JSON array of OAuth 2.0 response type strings + // that the client will restrict itself to using. + // If omitted, the default is ["code"]. + ResponseTypes []string `json:"response_types,omitempty"` + + // ClientName is a RECOMMENDED human-readable name of the client to be presented + // to the end-user. + ClientName string `json:"client_name,omitempty"` + + // ClientURI is a RECOMMENDED URL of a web page providing information about the client. + ClientURI string `json:"client_uri,omitempty"` + + // LogoURI is an OPTIONAL URL of a logo for the client, which may be displayed + // to the end-user. + LogoURI string `json:"logo_uri,omitempty"` + + // Scope is an OPTIONAL string containing a space-separated list of scope values + // that the client will restrict itself to using. + Scope string `json:"scope,omitempty"` + + // Contacts is an OPTIONAL JSON array of strings representing ways to contact + // people responsible for this client (e.g., email addresses). + Contacts []string `json:"contacts,omitempty"` + + // TOSURI is an OPTIONAL URL that the client provides to the end-user + // to read about the client's terms of service. + TOSURI string `json:"tos_uri,omitempty"` + + // PolicyURI is an OPTIONAL URL that the client provides to the end-user + // to read about the client's privacy policy. + PolicyURI string `json:"policy_uri,omitempty"` + + // JWKSURI is an OPTIONAL URL for the client's JSON Web Key Set [JWK] document. + // This is preferred over the 'jwks' parameter. + JWKSURI string `json:"jwks_uri,omitempty"` + + // JWKS is an OPTIONAL client's JSON Web Key Set [JWK] document, passed by value. + // This is an alternative to providing a JWKSURI. + JWKS string `json:"jwks,omitempty"` + + // SoftwareID is an OPTIONAL unique identifier string for the client software, + // constant across all instances and versions. + SoftwareID string `json:"software_id,omitempty"` + + // SoftwareVersion is an OPTIONAL version identifier string for the client software. + SoftwareVersion string `json:"software_version,omitempty"` + + // SoftwareStatement is an OPTIONAL JWT that asserts client metadata values. + // Values in the software statement take precedence over other metadata values. + SoftwareStatement string `json:"software_statement,omitempty"` +} + +// AuthClientInformation represents the fields returned by the Authorization Server +// (RFC 7591, Section 3.2.1 and 3.2.2). +type AuthClientInformation struct { + // AuthClientMetadata contains all registered client metadata, returned by the + // server on success, potentially with modified or defaulted values. + AuthClientMetadata + + // ClientID is the REQUIRED newly issued OAuth 2.0 client identifier. + ClientID string `json:"client_id"` + + // ClientSecret is an OPTIONAL client secret string. + ClientSecret string `json:"client_secret,omitempty"` + + // ClientIDIssuedAt is an OPTIONAL timestamp (seconds from 1970 UTC) when + // the ClientID was issued. + ClientIDIssuedAt int64 `json:"client_id_issued_at,omitempty"` + + // ClientSecretExpiresAt is the REQUIRED (if client_secret is issued) timestamp + // when the secret expires, or 0 if it never expires. + ClientSecretExpiresAt int64 `json:"client_secret_expires_at,omitempty"` +} + +// AuthClientRegistrationError is the error response from the Authorization Server +// for a failed registration attempt (RFC 7591, Section 3.2.2). +type AuthClientRegistrationError struct { + // Error is the REQUIRED error code if registration failed (RFC 7591, 3.2.2). + Error string `json:"error"` + + // ErrorDescription is an OPTIONAL human-readable error message. + ErrorDescription string `json:"error_description,omitempty"` +} + var wellKnownPaths = []string{ "/.well-known/oauth-authorization-server", "/.well-known/openid-configuration", @@ -123,7 +226,7 @@ var wellKnownPaths = []string{ // - The Issuer field is checked against issuerURL. // // [RFC 8414]: https://tools.ietf.org/html/rfc8414 -func GetAuthServerMeta(ctx context.Context, issuerURL string, c *http.Client) (*AuthServerMeta, error) { +func GetAuthServerMeta(ctx context.Context, issuerURL string, c *http.Client) (*AuthServerMetadata, error) { var errs []error for _, p := range wellKnownPaths { u, err := prependToPath(issuerURL, p) @@ -131,7 +234,7 @@ func GetAuthServerMeta(ctx context.Context, issuerURL string, c *http.Client) (* // issuerURL is bad; no point in continuing. return nil, err } - asm, err := getJSON[AuthServerMeta](ctx, c, u, 1<<20) + asm, err := getJSON[AuthServerMetadata](ctx, c, u, 1<<20) if err == nil { if asm.Issuer != issuerURL { // section 3.3 // Security violation; don't keep trying. @@ -143,3 +246,59 @@ func GetAuthServerMeta(ctx context.Context, issuerURL string, c *http.Client) (* } return nil, fmt.Errorf("failed to get auth server metadata from %q: %w", issuerURL, errors.Join(errs...)) } + +// RegisterClient performs Dynamic Client Registration according to RFC 7591. +func RegisterClient(ctx context.Context, serverMeta *AuthServerMetadata, clientMeta *AuthClientMetadata, c *http.Client) (*AuthClientInformation, error) { + if serverMeta == nil || serverMeta.RegistrationEndpoint == "" { + return nil, fmt.Errorf("server metadata does not contain a registration_endpoint") + } + + if c == nil { + c = http.DefaultClient + } + + payload, err := json.Marshal(clientMeta) + if err != nil { + return nil, fmt.Errorf("failed to marshal client metadata: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, "POST", serverMeta.RegistrationEndpoint, bytes.NewBuffer(payload)) + if err != nil { + return nil, fmt.Errorf("failed to create registration request: %w", err) + } + + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json") + + resp, err := c.Do(req) + if err != nil { + return nil, fmt.Errorf("registration request failed: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read registration response body: %w", err) + } + + if resp.StatusCode == http.StatusCreated { + var authClientInfo AuthClientInformation + if err := json.Unmarshal(body, &authClientInfo); err != nil { + return nil, fmt.Errorf("failed to decode successful registration response: %w (%s)", err, string(body)) + } + if authClientInfo.ClientID == "" { + return nil, fmt.Errorf("registration response is missing required 'client_id' field") + } + return &authClientInfo, nil + } + + if resp.StatusCode == http.StatusBadRequest { + var regError AuthClientRegistrationError + if err := json.Unmarshal(body, ®Error); err != nil { + return nil, fmt.Errorf("failed to decode registration error response: %w (%s)", err, string(body)) + } + return nil, fmt.Errorf("registration failed: %s (%s)", regError.Error, regError.ErrorDescription) + } + + return nil, fmt.Errorf("registration failed with status %s: %s", resp.Status, string(body)) +} diff --git a/internal/oauthex/auth_meta_test.go b/internal/oauthex/auth_meta_test.go index b83402f2..ef8a0082 100644 --- a/internal/oauthex/auth_meta_test.go +++ b/internal/oauthex/auth_meta_test.go @@ -5,19 +5,24 @@ package oauthex import ( + "context" "encoding/json" + "io" + "net/http" + "net/http/httptest" "os" "path/filepath" + "strings" "testing" ) -func TestAuthMetaParse(t *testing.T) { +func TestAuthServerMetadataParse(t *testing.T) { // Verify that we parse Google's auth server metadata. data, err := os.ReadFile(filepath.FromSlash("testdata/google-auth-meta.json")) if err != nil { t.Fatal(err) } - var a AuthServerMeta + var a AuthServerMetadata if err := json.Unmarshal(data, &a); err != nil { t.Fatal(err) } @@ -26,3 +31,125 @@ func TestAuthMetaParse(t *testing.T) { t.Errorf("got %q, want %q", g, w) } } + +func TestAuthClientMetadataParse(t *testing.T) { + // Verify that we can parse a typical client metadata JSON. + data, err := os.ReadFile(filepath.FromSlash("testdata/client-auth-meta.json")) + if err != nil { + t.Fatal(err) + } + var a AuthClientMetadata + if err := json.Unmarshal(data, &a); err != nil { + t.Fatal(err) + } + // Spot check + if g, w := a.ClientName, "My Test App"; g != w { + t.Errorf("got ClientName %q, want %q", g, w) + } + if g, w := len(a.RedirectURIs), 2; g != w { + t.Errorf("got %d RedirectURIs, want %d", g, w) + } +} + +func TestRegisterClient(t *testing.T) { + testCases := []struct { + name string + handler http.HandlerFunc + clientMeta *AuthClientMetadata + wantClientID string + wantErr string + }{ + { + name: "Success", + handler: func(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" { + t.Errorf("Expected POST, got %s", r.Method) + } + body, err := io.ReadAll(r.Body) + if err != nil { + t.Fatal(err) + } + var receivedMeta AuthClientMetadata + if err := json.Unmarshal(body, &receivedMeta); err != nil { + t.Fatalf("Failed to unmarshal request body: %v", err) + } + if receivedMeta.ClientName != "Test App" { + t.Errorf("Expected ClientName 'Test App', got '%s'", receivedMeta.ClientName) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"client_id":"test-client-id","client_secret":"test-client-secret","client_name":"Test App"}`)) + }, + clientMeta: &AuthClientMetadata{ClientName: "Test App", RedirectURIs: []string{"http://localhost/cb"}}, + wantClientID: "test-client-id", + }, + { + name: "Error - Missing ClientID in Response", + handler: func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{"client_secret":"test-client-secret"}`)) // No client_id + }, + clientMeta: &AuthClientMetadata{RedirectURIs: []string{"http://localhost/cb"}}, + wantErr: "registration response is missing required 'client_id' field", + }, + { + name: "Error - Standard OAuth Error", + handler: func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(`{"error":"invalid_redirect_uri","error_description":"Redirect URI is not valid."}`)) + }, + clientMeta: &AuthClientMetadata{RedirectURIs: []string{"http://invalid/cb"}}, + wantErr: "registration failed: invalid_redirect_uri (Redirect URI is not valid.)", + }, + { + name: "Error - Non-JSON Server Error", + handler: func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte("Internal Server Error")) + }, + clientMeta: &AuthClientMetadata{RedirectURIs: []string{"http://localhost/cb"}}, + wantErr: "registration failed with status 500 Internal Server Error", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + server := httptest.NewServer(tc.handler) + defer server.Close() + + serverMeta := &AuthServerMetadata{RegistrationEndpoint: server.URL} + info, err := RegisterClient(context.Background(), serverMeta, tc.clientMeta, server.Client()) + + if tc.wantErr != "" { + if err == nil { + t.Fatalf("Expected an error containing '%s', but got nil", tc.wantErr) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("Expected error to contain '%s', got '%v'", tc.wantErr, err) + } + return + } + + if err != nil { + t.Fatalf("Expected no error, but got: %v", err) + } + if info.ClientID != tc.wantClientID { + t.Errorf("Expected client_id '%s', got '%s'", tc.wantClientID, info.ClientID) + } + }) + } + + t.Run("Error - No Endpoint in Metadata", func(t *testing.T) { + serverMeta := &AuthServerMetadata{Issuer: "http://localhost"} // No RegistrationEndpoint + _, err := RegisterClient(context.Background(), serverMeta, &AuthClientMetadata{}, nil) + if err == nil { + t.Fatal("Expected an error for missing registration endpoint, got nil") + } + expectedErr := "server metadata does not contain a registration_endpoint" + if err.Error() != expectedErr { + t.Errorf("Expected error '%s', got '%v'", expectedErr, err) + } + }) +} diff --git a/internal/oauthex/testdata/client-auth-meta.json b/internal/oauthex/testdata/client-auth-meta.json new file mode 100644 index 00000000..b8bdc1e3 --- /dev/null +++ b/internal/oauthex/testdata/client-auth-meta.json @@ -0,0 +1,9 @@ +{ + "client_name": "My Test App", + "redirect_uris": [ + "https://client.example.org/callback", + "https://client.example.org/callback2" + ], + "scope": "read write" + } + \ No newline at end of file From 73c7b52b6982745ea72fe565cfa30f065b5433cf Mon Sep 17 00:00:00 2001 From: Sam Thanawalla Date: Tue, 23 Sep 2025 17:46:08 +0000 Subject: [PATCH 2/5] rename metadata to meta --- internal/oauthex/auth_meta.go | 18 ++++++------- internal/oauthex/auth_meta_test.go | 26 +++++++++---------- .../oauthex/testdata/client-auth-meta.json | 7 +++-- 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/internal/oauthex/auth_meta.go b/internal/oauthex/auth_meta.go index f91ac026..20408d70 100644 --- a/internal/oauthex/auth_meta.go +++ b/internal/oauthex/auth_meta.go @@ -17,14 +17,14 @@ import ( "net/http" ) -// AuthServerMetadata represents the metadata for an OAuth 2.0 authorization server, +// AuthServerMeta represents the metadata for an OAuth 2.0 authorization server, // as defined in [RFC 8414]. // // Not supported: // - signed metadata // // [RFC 8414]: https://tools.ietf.org/html/rfc8414) -type AuthServerMetadata struct { +type AuthServerMeta struct { // GENERATED BY GEMINI 2.5. // Issuer is the REQUIRED URL identifying the authorization server. @@ -112,8 +112,8 @@ type AuthServerMetadata struct { CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported,omitempty"` } -// AuthClientMetadata represents the client metadata fields for the DCR POST request (RFC 7591). -type AuthClientMetadata struct { +// AuthClientMeta represents the client metadata fields for the DCR POST request (RFC 7591). +type AuthClientMeta struct { // RedirectURIs is a REQUIRED JSON array of redirection URI strings for use in // redirect-based flows (such as the authorization code grant). RedirectURIs []string `json:"redirect_uris"` @@ -183,9 +183,9 @@ type AuthClientMetadata struct { // AuthClientInformation represents the fields returned by the Authorization Server // (RFC 7591, Section 3.2.1 and 3.2.2). type AuthClientInformation struct { - // AuthClientMetadata contains all registered client metadata, returned by the + // AuthClientMeta contains all registered client metadata, returned by the // server on success, potentially with modified or defaulted values. - AuthClientMetadata + AuthClientMeta // ClientID is the REQUIRED newly issued OAuth 2.0 client identifier. ClientID string `json:"client_id"` @@ -226,7 +226,7 @@ var wellKnownPaths = []string{ // - The Issuer field is checked against issuerURL. // // [RFC 8414]: https://tools.ietf.org/html/rfc8414 -func GetAuthServerMeta(ctx context.Context, issuerURL string, c *http.Client) (*AuthServerMetadata, error) { +func GetAuthServerMeta(ctx context.Context, issuerURL string, c *http.Client) (*AuthServerMeta, error) { var errs []error for _, p := range wellKnownPaths { u, err := prependToPath(issuerURL, p) @@ -234,7 +234,7 @@ func GetAuthServerMeta(ctx context.Context, issuerURL string, c *http.Client) (* // issuerURL is bad; no point in continuing. return nil, err } - asm, err := getJSON[AuthServerMetadata](ctx, c, u, 1<<20) + asm, err := getJSON[AuthServerMeta](ctx, c, u, 1<<20) if err == nil { if asm.Issuer != issuerURL { // section 3.3 // Security violation; don't keep trying. @@ -248,7 +248,7 @@ func GetAuthServerMeta(ctx context.Context, issuerURL string, c *http.Client) (* } // RegisterClient performs Dynamic Client Registration according to RFC 7591. -func RegisterClient(ctx context.Context, serverMeta *AuthServerMetadata, clientMeta *AuthClientMetadata, c *http.Client) (*AuthClientInformation, error) { +func RegisterClient(ctx context.Context, serverMeta *AuthServerMeta, clientMeta *AuthClientMeta, c *http.Client) (*AuthClientInformation, error) { if serverMeta == nil || serverMeta.RegistrationEndpoint == "" { return nil, fmt.Errorf("server metadata does not contain a registration_endpoint") } diff --git a/internal/oauthex/auth_meta_test.go b/internal/oauthex/auth_meta_test.go index ef8a0082..9b30a5b8 100644 --- a/internal/oauthex/auth_meta_test.go +++ b/internal/oauthex/auth_meta_test.go @@ -16,13 +16,13 @@ import ( "testing" ) -func TestAuthServerMetadataParse(t *testing.T) { +func TestAuthServerMetaParse(t *testing.T) { // Verify that we parse Google's auth server metadata. data, err := os.ReadFile(filepath.FromSlash("testdata/google-auth-meta.json")) if err != nil { t.Fatal(err) } - var a AuthServerMetadata + var a AuthServerMeta if err := json.Unmarshal(data, &a); err != nil { t.Fatal(err) } @@ -32,13 +32,13 @@ func TestAuthServerMetadataParse(t *testing.T) { } } -func TestAuthClientMetadataParse(t *testing.T) { +func TestAuthClientMetaParse(t *testing.T) { // Verify that we can parse a typical client metadata JSON. data, err := os.ReadFile(filepath.FromSlash("testdata/client-auth-meta.json")) if err != nil { t.Fatal(err) } - var a AuthClientMetadata + var a AuthClientMeta if err := json.Unmarshal(data, &a); err != nil { t.Fatal(err) } @@ -55,7 +55,7 @@ func TestRegisterClient(t *testing.T) { testCases := []struct { name string handler http.HandlerFunc - clientMeta *AuthClientMetadata + clientMeta *AuthClientMeta wantClientID string wantErr string }{ @@ -69,7 +69,7 @@ func TestRegisterClient(t *testing.T) { if err != nil { t.Fatal(err) } - var receivedMeta AuthClientMetadata + var receivedMeta AuthClientMeta if err := json.Unmarshal(body, &receivedMeta); err != nil { t.Fatalf("Failed to unmarshal request body: %v", err) } @@ -80,7 +80,7 @@ func TestRegisterClient(t *testing.T) { w.WriteHeader(http.StatusCreated) w.Write([]byte(`{"client_id":"test-client-id","client_secret":"test-client-secret","client_name":"Test App"}`)) }, - clientMeta: &AuthClientMetadata{ClientName: "Test App", RedirectURIs: []string{"http://localhost/cb"}}, + clientMeta: &AuthClientMeta{ClientName: "Test App", RedirectURIs: []string{"http://localhost/cb"}}, wantClientID: "test-client-id", }, { @@ -90,7 +90,7 @@ func TestRegisterClient(t *testing.T) { w.WriteHeader(http.StatusCreated) w.Write([]byte(`{"client_secret":"test-client-secret"}`)) // No client_id }, - clientMeta: &AuthClientMetadata{RedirectURIs: []string{"http://localhost/cb"}}, + clientMeta: &AuthClientMeta{RedirectURIs: []string{"http://localhost/cb"}}, wantErr: "registration response is missing required 'client_id' field", }, { @@ -100,7 +100,7 @@ func TestRegisterClient(t *testing.T) { w.WriteHeader(http.StatusBadRequest) w.Write([]byte(`{"error":"invalid_redirect_uri","error_description":"Redirect URI is not valid."}`)) }, - clientMeta: &AuthClientMetadata{RedirectURIs: []string{"http://invalid/cb"}}, + clientMeta: &AuthClientMeta{RedirectURIs: []string{"http://invalid/cb"}}, wantErr: "registration failed: invalid_redirect_uri (Redirect URI is not valid.)", }, { @@ -109,7 +109,7 @@ func TestRegisterClient(t *testing.T) { w.WriteHeader(http.StatusInternalServerError) w.Write([]byte("Internal Server Error")) }, - clientMeta: &AuthClientMetadata{RedirectURIs: []string{"http://localhost/cb"}}, + clientMeta: &AuthClientMeta{RedirectURIs: []string{"http://localhost/cb"}}, wantErr: "registration failed with status 500 Internal Server Error", }, } @@ -119,7 +119,7 @@ func TestRegisterClient(t *testing.T) { server := httptest.NewServer(tc.handler) defer server.Close() - serverMeta := &AuthServerMetadata{RegistrationEndpoint: server.URL} + serverMeta := &AuthServerMeta{RegistrationEndpoint: server.URL} info, err := RegisterClient(context.Background(), serverMeta, tc.clientMeta, server.Client()) if tc.wantErr != "" { @@ -142,8 +142,8 @@ func TestRegisterClient(t *testing.T) { } t.Run("Error - No Endpoint in Metadata", func(t *testing.T) { - serverMeta := &AuthServerMetadata{Issuer: "http://localhost"} // No RegistrationEndpoint - _, err := RegisterClient(context.Background(), serverMeta, &AuthClientMetadata{}, nil) + serverMeta := &AuthServerMeta{Issuer: "http://localhost"} // No RegistrationEndpoint + _, err := RegisterClient(context.Background(), serverMeta, &AuthClientMeta{}, nil) if err == nil { t.Fatal("Expected an error for missing registration endpoint, got nil") } diff --git a/internal/oauthex/testdata/client-auth-meta.json b/internal/oauthex/testdata/client-auth-meta.json index b8bdc1e3..c07f5be1 100644 --- a/internal/oauthex/testdata/client-auth-meta.json +++ b/internal/oauthex/testdata/client-auth-meta.json @@ -1,9 +1,8 @@ { "client_name": "My Test App", "redirect_uris": [ - "https://client.example.org/callback", - "https://client.example.org/callback2" + "https://client.example.org/callback", + "https://client.example.org/callback2" ], "scope": "read write" - } - \ No newline at end of file +} From d4ee5e9712289efade5a0b8cbdce0ec395ba9c9e Mon Sep 17 00:00:00 2001 From: Sam Thanawalla Date: Wed, 24 Sep 2025 17:30:39 +0000 Subject: [PATCH 3/5] rename structs --- internal/oauthex/auth_meta.go | 44 ++++++++++++++++-------------- internal/oauthex/auth_meta_test.go | 34 +++++++++++------------ 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/internal/oauthex/auth_meta.go b/internal/oauthex/auth_meta.go index 20408d70..4061d684 100644 --- a/internal/oauthex/auth_meta.go +++ b/internal/oauthex/auth_meta.go @@ -112,8 +112,8 @@ type AuthServerMeta struct { CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported,omitempty"` } -// AuthClientMeta represents the client metadata fields for the DCR POST request (RFC 7591). -type AuthClientMeta struct { +// ClientRegistrationMetadata represents the client metadata fields for the DCR POST request (RFC 7591). +type ClientRegistrationMetadata struct { // RedirectURIs is a REQUIRED JSON array of redirection URI strings for use in // redirect-based flows (such as the authorization code grant). RedirectURIs []string `json:"redirect_uris"` @@ -180,12 +180,12 @@ type AuthClientMeta struct { SoftwareStatement string `json:"software_statement,omitempty"` } -// AuthClientInformation represents the fields returned by the Authorization Server +// ClientRegistrationResponse represents the fields returned by the Authorization Server // (RFC 7591, Section 3.2.1 and 3.2.2). -type AuthClientInformation struct { - // AuthClientMeta contains all registered client metadata, returned by the +type ClientRegistrationResponse struct { + // ClientRegistrationMetadata contains all registered client metadata, returned by the // server on success, potentially with modified or defaulted values. - AuthClientMeta + ClientRegistrationMetadata // ClientID is the REQUIRED newly issued OAuth 2.0 client identifier. ClientID string `json:"client_id"` @@ -202,16 +202,20 @@ type AuthClientInformation struct { ClientSecretExpiresAt int64 `json:"client_secret_expires_at,omitempty"` } -// AuthClientRegistrationError is the error response from the Authorization Server +// ClientRegistrationError is the error response from the Authorization Server // for a failed registration attempt (RFC 7591, Section 3.2.2). -type AuthClientRegistrationError struct { - // Error is the REQUIRED error code if registration failed (RFC 7591, 3.2.2). - Error string `json:"error"` +type ClientRegistrationError struct { + // ErrorCode is the REQUIRED error code if registration failed (RFC 7591, 3.2.2). + ErrorCode string `json:"error"` // ErrorDescription is an OPTIONAL human-readable error message. ErrorDescription string `json:"error_description,omitempty"` } +func (e *ClientRegistrationError) Error() string { + return fmt.Sprintf("registration failed: %s (%s)", e.ErrorCode, e.ErrorDescription) +} + var wellKnownPaths = []string{ "/.well-known/oauth-authorization-server", "/.well-known/openid-configuration", @@ -248,9 +252,9 @@ func GetAuthServerMeta(ctx context.Context, issuerURL string, c *http.Client) (* } // RegisterClient performs Dynamic Client Registration according to RFC 7591. -func RegisterClient(ctx context.Context, serverMeta *AuthServerMeta, clientMeta *AuthClientMeta, c *http.Client) (*AuthClientInformation, error) { - if serverMeta == nil || serverMeta.RegistrationEndpoint == "" { - return nil, fmt.Errorf("server metadata does not contain a registration_endpoint") +func RegisterClient(ctx context.Context, registrationEndpoint string, clientMeta *ClientRegistrationMetadata, c *http.Client) (*ClientRegistrationResponse, error) { + if registrationEndpoint == "" { + return nil, fmt.Errorf("registration_endpoint is required") } if c == nil { @@ -262,7 +266,7 @@ func RegisterClient(ctx context.Context, serverMeta *AuthServerMeta, clientMeta return nil, fmt.Errorf("failed to marshal client metadata: %w", err) } - req, err := http.NewRequestWithContext(ctx, "POST", serverMeta.RegistrationEndpoint, bytes.NewBuffer(payload)) + req, err := http.NewRequestWithContext(ctx, "POST", registrationEndpoint, bytes.NewBuffer(payload)) if err != nil { return nil, fmt.Errorf("failed to create registration request: %w", err) } @@ -282,22 +286,22 @@ func RegisterClient(ctx context.Context, serverMeta *AuthServerMeta, clientMeta } if resp.StatusCode == http.StatusCreated { - var authClientInfo AuthClientInformation - if err := json.Unmarshal(body, &authClientInfo); err != nil { + var regResponse ClientRegistrationResponse + if err := json.Unmarshal(body, ®Response); err != nil { return nil, fmt.Errorf("failed to decode successful registration response: %w (%s)", err, string(body)) } - if authClientInfo.ClientID == "" { + if regResponse.ClientID == "" { return nil, fmt.Errorf("registration response is missing required 'client_id' field") } - return &authClientInfo, nil + return ®Response, nil } if resp.StatusCode == http.StatusBadRequest { - var regError AuthClientRegistrationError + var regError ClientRegistrationError if err := json.Unmarshal(body, ®Error); err != nil { return nil, fmt.Errorf("failed to decode registration error response: %w (%s)", err, string(body)) } - return nil, fmt.Errorf("registration failed: %s (%s)", regError.Error, regError.ErrorDescription) + return nil, ®Error } return nil, fmt.Errorf("registration failed with status %s: %s", resp.Status, string(body)) diff --git a/internal/oauthex/auth_meta_test.go b/internal/oauthex/auth_meta_test.go index 9b30a5b8..b9000df2 100644 --- a/internal/oauthex/auth_meta_test.go +++ b/internal/oauthex/auth_meta_test.go @@ -16,7 +16,7 @@ import ( "testing" ) -func TestAuthServerMetaParse(t *testing.T) { +func TestAuthMetaParse(t *testing.T) { // Verify that we parse Google's auth server metadata. data, err := os.ReadFile(filepath.FromSlash("testdata/google-auth-meta.json")) if err != nil { @@ -32,13 +32,13 @@ func TestAuthServerMetaParse(t *testing.T) { } } -func TestAuthClientMetaParse(t *testing.T) { +func TestClientRegistrationMetadataParse(t *testing.T) { // Verify that we can parse a typical client metadata JSON. data, err := os.ReadFile(filepath.FromSlash("testdata/client-auth-meta.json")) if err != nil { t.Fatal(err) } - var a AuthClientMeta + var a ClientRegistrationMetadata if err := json.Unmarshal(data, &a); err != nil { t.Fatal(err) } @@ -55,7 +55,7 @@ func TestRegisterClient(t *testing.T) { testCases := []struct { name string handler http.HandlerFunc - clientMeta *AuthClientMeta + clientMeta *ClientRegistrationMetadata wantClientID string wantErr string }{ @@ -69,7 +69,7 @@ func TestRegisterClient(t *testing.T) { if err != nil { t.Fatal(err) } - var receivedMeta AuthClientMeta + var receivedMeta ClientRegistrationMetadata if err := json.Unmarshal(body, &receivedMeta); err != nil { t.Fatalf("Failed to unmarshal request body: %v", err) } @@ -80,36 +80,36 @@ func TestRegisterClient(t *testing.T) { w.WriteHeader(http.StatusCreated) w.Write([]byte(`{"client_id":"test-client-id","client_secret":"test-client-secret","client_name":"Test App"}`)) }, - clientMeta: &AuthClientMeta{ClientName: "Test App", RedirectURIs: []string{"http://localhost/cb"}}, + clientMeta: &ClientRegistrationMetadata{ClientName: "Test App", RedirectURIs: []string{"http://localhost/cb"}}, wantClientID: "test-client-id", }, { - name: "Error - Missing ClientID in Response", + name: "Missing ClientID in Response", handler: func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusCreated) w.Write([]byte(`{"client_secret":"test-client-secret"}`)) // No client_id }, - clientMeta: &AuthClientMeta{RedirectURIs: []string{"http://localhost/cb"}}, + clientMeta: &ClientRegistrationMetadata{RedirectURIs: []string{"http://localhost/cb"}}, wantErr: "registration response is missing required 'client_id' field", }, { - name: "Error - Standard OAuth Error", + name: "Standard OAuth Error", handler: func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusBadRequest) w.Write([]byte(`{"error":"invalid_redirect_uri","error_description":"Redirect URI is not valid."}`)) }, - clientMeta: &AuthClientMeta{RedirectURIs: []string{"http://invalid/cb"}}, + clientMeta: &ClientRegistrationMetadata{RedirectURIs: []string{"http://invalid/cb"}}, wantErr: "registration failed: invalid_redirect_uri (Redirect URI is not valid.)", }, { - name: "Error - Non-JSON Server Error", + name: "Non-JSON Server Error", handler: func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) w.Write([]byte("Internal Server Error")) }, - clientMeta: &AuthClientMeta{RedirectURIs: []string{"http://localhost/cb"}}, + clientMeta: &ClientRegistrationMetadata{RedirectURIs: []string{"http://localhost/cb"}}, wantErr: "registration failed with status 500 Internal Server Error", }, } @@ -119,8 +119,7 @@ func TestRegisterClient(t *testing.T) { server := httptest.NewServer(tc.handler) defer server.Close() - serverMeta := &AuthServerMeta{RegistrationEndpoint: server.URL} - info, err := RegisterClient(context.Background(), serverMeta, tc.clientMeta, server.Client()) + info, err := RegisterClient(context.Background(), server.URL, tc.clientMeta, server.Client()) if tc.wantErr != "" { if err == nil { @@ -141,13 +140,12 @@ func TestRegisterClient(t *testing.T) { }) } - t.Run("Error - No Endpoint in Metadata", func(t *testing.T) { - serverMeta := &AuthServerMeta{Issuer: "http://localhost"} // No RegistrationEndpoint - _, err := RegisterClient(context.Background(), serverMeta, &AuthClientMeta{}, nil) + t.Run("No Endpoint", func(t *testing.T) { + _, err := RegisterClient(context.Background(), "", &ClientRegistrationMetadata{}, nil) if err == nil { t.Fatal("Expected an error for missing registration endpoint, got nil") } - expectedErr := "server metadata does not contain a registration_endpoint" + expectedErr := "registration_endpoint is required" if err.Error() != expectedErr { t.Errorf("Expected error '%s', got '%v'", expectedErr, err) } From 8f7b3166d3ed6ba218a0a8c50cc93bff3ee494cc Mon Sep 17 00:00:00 2001 From: Sam Thanawalla Date: Thu, 25 Sep 2025 16:27:53 +0000 Subject: [PATCH 4/5] use time.Time --- internal/oauthex/auth_meta.go | 56 ++++++++++++++++++--- internal/oauthex/auth_meta_test.go | 78 ++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 6 deletions(-) diff --git a/internal/oauthex/auth_meta.go b/internal/oauthex/auth_meta.go index 4061d684..8b3675ef 100644 --- a/internal/oauthex/auth_meta.go +++ b/internal/oauthex/auth_meta.go @@ -15,6 +15,7 @@ import ( "fmt" "io" "net/http" + "time" ) // AuthServerMeta represents the metadata for an OAuth 2.0 authorization server, @@ -193,13 +194,56 @@ type ClientRegistrationResponse struct { // ClientSecret is an OPTIONAL client secret string. ClientSecret string `json:"client_secret,omitempty"` - // ClientIDIssuedAt is an OPTIONAL timestamp (seconds from 1970 UTC) when - // the ClientID was issued. - ClientIDIssuedAt int64 `json:"client_id_issued_at,omitempty"` + // ClientIDIssuedAt is an OPTIONAL Unix timestamp when the ClientID was issued. + ClientIDIssuedAt time.Time `json:"client_id_issued_at,omitempty"` - // ClientSecretExpiresAt is the REQUIRED (if client_secret is issued) timestamp - // when the secret expires, or 0 if it never expires. - ClientSecretExpiresAt int64 `json:"client_secret_expires_at,omitempty"` + // ClientSecretExpiresAt is the REQUIRED (if client_secret is issued) Unix + // timestamp when the secret expires, or 0 if it never expires. + ClientSecretExpiresAt time.Time `json:"client_secret_expires_at,omitempty"` +} + +func (r ClientRegistrationResponse) MarshalJSON() ([]byte, error) { + type Alias ClientRegistrationResponse + var clientIDIssuedAt int64 + var clientSecretExpiresAt int64 + + if !r.ClientIDIssuedAt.IsZero() { + clientIDIssuedAt = r.ClientIDIssuedAt.Unix() + } + if !r.ClientSecretExpiresAt.IsZero() { + clientSecretExpiresAt = r.ClientSecretExpiresAt.Unix() + } + + return json.Marshal(&struct { + ClientIDIssuedAt int64 `json:"client_id_issued_at,omitempty"` + ClientSecretExpiresAt int64 `json:"client_secret_expires_at,omitempty"` + *Alias + }{ + ClientIDIssuedAt: clientIDIssuedAt, + ClientSecretExpiresAt: clientSecretExpiresAt, + Alias: (*Alias)(&r), + }) +} + +func (r *ClientRegistrationResponse) UnmarshalJSON(data []byte) error { + type Alias ClientRegistrationResponse + aux := &struct { + ClientIDIssuedAt int64 `json:"client_id_issued_at,omitempty"` + ClientSecretExpiresAt int64 `json:"client_secret_expires_at,omitempty"` + *Alias + }{ + Alias: (*Alias)(r), + } + if err := json.Unmarshal(data, &aux); err != nil { + return err + } + if aux.ClientIDIssuedAt != 0 { + r.ClientIDIssuedAt = time.Unix(aux.ClientIDIssuedAt, 0) + } + if aux.ClientSecretExpiresAt != 0 { + r.ClientSecretExpiresAt = time.Unix(aux.ClientSecretExpiresAt, 0) + } + return nil } // ClientRegistrationError is the error response from the Authorization Server diff --git a/internal/oauthex/auth_meta_test.go b/internal/oauthex/auth_meta_test.go index b9000df2..3e63b73d 100644 --- a/internal/oauthex/auth_meta_test.go +++ b/internal/oauthex/auth_meta_test.go @@ -14,6 +14,9 @@ import ( "path/filepath" "strings" "testing" + "time" + + "github.com/google/go-cmp/cmp" ) func TestAuthMetaParse(t *testing.T) { @@ -151,3 +154,78 @@ func TestRegisterClient(t *testing.T) { } }) } + +func TestClientRegistrationResponseJSON(t *testing.T) { + testCases := []struct { + name string + in ClientRegistrationResponse + wantJSON string + }{ + { + name: "full response", + in: ClientRegistrationResponse{ + ClientID: "test-client-id", + ClientSecret: "test-client-secret", + ClientIDIssuedAt: time.Unix(1758840047, 0), + ClientSecretExpiresAt: time.Unix(1790376047, 0), + }, + wantJSON: `{"client_id":"test-client-id","client_secret":"test-client-secret","client_id_issued_at":1758840047,"client_secret_expires_at":1790376047, "redirect_uris": null}`, + }, + { + name: "minimal response with only required fields", + in: ClientRegistrationResponse{ + ClientID: "test-client-id-minimal", + }, + wantJSON: `{"client_id":"test-client-id-minimal", "redirect_uris":null}`, + }, + { + name: "response with a secret that does not expire", + in: ClientRegistrationResponse{ + ClientID: "test-client-id-no-expiry", + ClientSecret: "test-secret-no-expiry", + }, + wantJSON: `{"client_id":"test-client-id-no-expiry","client_secret":"test-secret-no-expiry", "redirect_uris":null}`, + }, + { + name: "unmarshal with zero timestamp", + in: ClientRegistrationResponse{ClientID: "client-id-zero"}, + wantJSON: `{"client_id":"client-id-zero", "redirect_uris":null}`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Test MarshalJSON + t.Run("marshal", func(t *testing.T) { + b, err := json.Marshal(tc.in) + if err != nil { + t.Fatalf("Marshal() error = %v", err) + } + + var gotMap, wantMap map[string]any + if err := json.Unmarshal(b, &gotMap); err != nil { + t.Fatalf("failed to unmarshal actual result: %v", err) + } + if err := json.Unmarshal([]byte(tc.wantJSON), &wantMap); err != nil { + t.Fatalf("failed to unmarshal expected result: %v", err) + } + + if diff := cmp.Diff(wantMap, gotMap); diff != "" { + t.Errorf("Marshal() mismatch (-want +got):\n%s", diff) + } + }) + + // Test UnmarshalJSON + t.Run("unmarshal", func(t *testing.T) { + var got ClientRegistrationResponse + if err := json.Unmarshal([]byte(tc.wantJSON), &got); err != nil { + t.Fatalf("Unmarshal() error = %v", err) + } + + if diff := cmp.Diff(tc.in, got); diff != "" { + t.Errorf("Unmarshal() mismatch (-want +got):\n%s", diff) + } + }) + }) + } +} From 0a07c820a0545b3c29abbdf748d171aa24189e1a Mon Sep 17 00:00:00 2001 From: Sam Thanawalla Date: Thu, 25 Sep 2025 17:25:07 +0000 Subject: [PATCH 5/5] address comments --- internal/oauthex/auth_meta.go | 14 +++++++------- internal/oauthex/auth_meta_test.go | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/oauthex/auth_meta.go b/internal/oauthex/auth_meta.go index 8b3675ef..5bbbb412 100644 --- a/internal/oauthex/auth_meta.go +++ b/internal/oauthex/auth_meta.go @@ -202,8 +202,8 @@ type ClientRegistrationResponse struct { ClientSecretExpiresAt time.Time `json:"client_secret_expires_at,omitempty"` } -func (r ClientRegistrationResponse) MarshalJSON() ([]byte, error) { - type Alias ClientRegistrationResponse +func (r *ClientRegistrationResponse) MarshalJSON() ([]byte, error) { + type alias ClientRegistrationResponse var clientIDIssuedAt int64 var clientSecretExpiresAt int64 @@ -217,22 +217,22 @@ func (r ClientRegistrationResponse) MarshalJSON() ([]byte, error) { return json.Marshal(&struct { ClientIDIssuedAt int64 `json:"client_id_issued_at,omitempty"` ClientSecretExpiresAt int64 `json:"client_secret_expires_at,omitempty"` - *Alias + *alias }{ ClientIDIssuedAt: clientIDIssuedAt, ClientSecretExpiresAt: clientSecretExpiresAt, - Alias: (*Alias)(&r), + alias: (*alias)(r), }) } func (r *ClientRegistrationResponse) UnmarshalJSON(data []byte) error { - type Alias ClientRegistrationResponse + type alias ClientRegistrationResponse aux := &struct { ClientIDIssuedAt int64 `json:"client_id_issued_at,omitempty"` ClientSecretExpiresAt int64 `json:"client_secret_expires_at,omitempty"` - *Alias + *alias }{ - Alias: (*Alias)(r), + alias: (*alias)(r), } if err := json.Unmarshal(data, &aux); err != nil { return err diff --git a/internal/oauthex/auth_meta_test.go b/internal/oauthex/auth_meta_test.go index 3e63b73d..6ff9f3dd 100644 --- a/internal/oauthex/auth_meta_test.go +++ b/internal/oauthex/auth_meta_test.go @@ -197,7 +197,7 @@ func TestClientRegistrationResponseJSON(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Test MarshalJSON t.Run("marshal", func(t *testing.T) { - b, err := json.Marshal(tc.in) + b, err := json.Marshal(&tc.in) if err != nil { t.Fatalf("Marshal() error = %v", err) }