diff --git a/flake.nix b/flake.nix index edf75b5..fcad099 100644 --- a/flake.nix +++ b/flake.nix @@ -12,15 +12,25 @@ devShells = forAllSystems (system: let pkgs = nixpkgs.legacyPackages.${system}; - ci = pkgs.writeShellScriptBin "ci" '' + goarrange = pkgs.buildGoModule { + pname = "goarrange"; + version = "1.0.0"; + src = pkgs.fetchFromGitHub { + owner = "jdeflander"; + repo = "goarrange"; + rev = "v1.0.0"; + hash = "sha256-V03BgTeWcAspMHGUHlAgSbiTaoZ42hgb/Zb/yqZ2m+k="; + }; + vendorHash = "sha256-Xhxfiw1WeXFHrYIYvUytEtMzMbSxOrignmUC5kVna0o="; + }; + lint = pkgs.writeShellScriptBin "lint" '' set -euo pipefail - echo "--- test ---" - go test -v ./... + echo "--- format ---" + go fmt ./... + goarrange run -r + git diff --quiet echo "--- lint ---" golangci-lint run -E misspell,godot,whitespace ./... - echo "--- arrange ---" - command -v goarrange >/dev/null || go install github.com/jdeflander/goarrange@v1.0.0 - test -z "$(goarrange run -r -d)" echo "--- tidy ---" go mod tidy git diff --quiet go.mod go.sum @@ -31,7 +41,8 @@ packages = [ pkgs.go_1_26 pkgs.golangci-lint - ci + goarrange + lint ]; }; } diff --git a/handlers_test.go b/handlers_test.go index 4f002d2..ac34f13 100644 --- a/handlers_test.go +++ b/handlers_test.go @@ -91,6 +91,87 @@ func TestInvalidRequests(t *testing.T) { } } +func TestServerExplicitStatusCodes(t *testing.T) { + tests := []struct { + name string + method string + target string + body io.Reader + expectedStatus int + }{ + { + name: "GET resource", + method: http.MethodGet, + target: "/Users/0001", + expectedStatus: http.StatusOK, + }, + { + name: "GET resources", + method: http.MethodGet, + target: "/Users", + expectedStatus: http.StatusOK, + }, + { + name: "POST resource", + method: http.MethodPost, + target: "/Users", + body: strings.NewReader(`{"userName": "test", "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"]}`), + expectedStatus: http.StatusCreated, + }, + { + name: "PUT resource", + method: http.MethodPut, + target: "/Users/0001", + body: strings.NewReader(`{"userName": "test_replace", "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"]}`), + expectedStatus: http.StatusOK, + }, + { + name: "DELETE resource", + method: http.MethodDelete, + target: "/Users/0001", + expectedStatus: http.StatusNoContent, + }, + { + name: "GET ResourceTypes", + method: http.MethodGet, + target: "/ResourceTypes", + expectedStatus: http.StatusOK, + }, + { + name: "GET ResourceType", + method: http.MethodGet, + target: "/ResourceTypes/User", + expectedStatus: http.StatusOK, + }, + { + name: "GET Schemas", + method: http.MethodGet, + target: "/Schemas", + expectedStatus: http.StatusOK, + }, + { + name: "GET ServiceProviderConfig", + method: http.MethodGet, + target: "/ServiceProviderConfig", + expectedStatus: http.StatusOK, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(tt.method, tt.target, tt.body) + rr := httptest.NewRecorder() + w := &statusRecordingResponseWriter{ResponseWriter: rr} + newTestServer(t).ServeHTTP(w, req) + + if !w.calledWriteHeader { + t.Error("handler did not explicitly call WriteHeader") + } + assertEqualStatusCode(t, tt.expectedStatus, w.status) + }) + } +} + func TestServerMeEndpoint(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/Me", nil) rr := httptest.NewRecorder() @@ -1034,3 +1115,17 @@ func newTestServer(t *testing.T) Server { } return s } + +// statusRecordingResponseWriter wraps an http.ResponseWriter and records +// whether WriteHeader was called explicitly, simulating logging middleware. +type statusRecordingResponseWriter struct { + http.ResponseWriter + calledWriteHeader bool + status int +} + +func (w *statusRecordingResponseWriter) WriteHeader(status int) { + w.calledWriteHeader = true + w.status = status + w.ResponseWriter.WriteHeader(status) +} diff --git a/schema/schemas.go b/schema/schemas.go index a966f3f..17c40c4 100644 --- a/schema/schemas.go +++ b/schema/schemas.go @@ -697,9 +697,9 @@ func ServiceProviderConfigSchema() Schema { Attributes: []CoreAttribute{ SimpleCoreAttribute(SimpleStringParams(StringParams{ Description: optional.NewString("An HTTP-addressable URL pointing to the service provider's human-consumable help documentation."), - Mutability: AttributeMutabilityReadOnly(), - Name: "documentationUri", - Required: false, + Mutability: AttributeMutabilityReadOnly(), + Name: "documentationUri", + Required: false, })), ComplexCoreAttribute(ComplexParams{ Description: optional.NewString("A complex type that specifies PATCH configuration options."), @@ -763,9 +763,9 @@ func ServiceProviderConfigSchema() Schema { }), ComplexCoreAttribute(ComplexParams{ Description: optional.NewString("A complex type that specifies configuration options related to changing a password."), - Mutability: AttributeMutabilityReadOnly(), - Name: "changePassword", - Required: true, + Mutability: AttributeMutabilityReadOnly(), + Name: "changePassword", + Required: true, SubAttributes: []SimpleParams{ SimpleBooleanParams(BooleanParams{ Description: optional.NewString("A Boolean value specifying whether or not the operation is supported."), @@ -812,9 +812,9 @@ func ServiceProviderConfigSchema() Schema { SubAttributes: []SimpleParams{ SimpleStringParams(StringParams{ Description: optional.NewString("The authentication scheme. This specification defines the values 'oauth', 'oauth2', 'oauthbearertoken', 'httpbasic', and 'httpdigest'."), - Mutability: AttributeMutabilityReadOnly(), - Name: "type", - Required: true, + Mutability: AttributeMutabilityReadOnly(), + Name: "type", + Required: true, }), SimpleStringParams(StringParams{ Description: optional.NewString("The common authentication scheme name, e.g., HTTP Basic."), @@ -830,8 +830,8 @@ func ServiceProviderConfigSchema() Schema { }), SimpleStringParams(StringParams{ Description: optional.NewString("An HTTP-addressable URL pointing to the authentication scheme's specification."), - Mutability: AttributeMutabilityReadOnly(), - Name: "specUri", + Mutability: AttributeMutabilityReadOnly(), + Name: "specUri", }), SimpleStringParams(StringParams{ Description: optional.NewString("An HTTP-addressable URL pointing to the authentication scheme's usage documentation."), diff --git a/server.go b/server.go index c701bcc..a13f5b0 100644 --- a/server.go +++ b/server.go @@ -89,6 +89,7 @@ func NewServer(args *ServerArgs, opts ...ServerOption) (Server, error) { // ServeHTTP dispatches the request to the handler whose pattern most closely matches the request URL. func (s Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/scim+json") + w = &statusResponseWriter{ResponseWriter: w} path := strings.TrimPrefix(r.URL.Path, "/v2") @@ -257,3 +258,26 @@ func WithLogger(logger Logger) ServerOption { } } } + +// statusResponseWriter wraps http.ResponseWriter to ensure WriteHeader is +// always called explicitly. If Write is called without a prior WriteHeader, +// it defaults to http.StatusOK. Subsequent WriteHeader calls are ignored. +// This allows observability middleware to reliably capture status codes. +type statusResponseWriter struct { + http.ResponseWriter + wroteHeader bool +} + +func (w *statusResponseWriter) Write(b []byte) (int, error) { + if !w.wroteHeader { + w.WriteHeader(http.StatusOK) + } + return w.ResponseWriter.Write(b) +} + +func (w *statusResponseWriter) WriteHeader(status int) { + if !w.wroteHeader { + w.wroteHeader = true + w.ResponseWriter.WriteHeader(status) + } +}