diff --git a/management/server/http/handlers/setup_keys/setupkeys_handler.go b/management/server/http/handlers/setup_keys/setupkeys_handler.go index d267b6eea..bb498f46b 100644 --- a/management/server/http/handlers/setup_keys/setupkeys_handler.go +++ b/management/server/http/handlers/setup_keys/setupkeys_handler.go @@ -64,6 +64,19 @@ func (h *handler) createSetupKey(w http.ResponseWriter, r *http.Request) { return } + // A one-off key can be used once, and GenerateSetupKey pins its usage limit + // at 1 whatever the request says. Silently overriding a caller that asked + // for a different number leaves them holding a key that does not do what + // they configured, and no way to find out except by using it. Only values + // above 1 are refused: usage_limit is a required field with no null, so 0 + // cannot be told apart from a caller that has nothing to say about it. + if types.SetupKeyType(req.Type) == types.SetupKeyOneOff && req.UsageLimit > 1 { + util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, + "usage_limit %d is not valid for a one-off setup key, which can be used once; use type reusable for a key that can be used more than once", + req.UsageLimit), w) + return + } + expiresIn := time.Duration(req.ExpiresIn) * time.Second if expiresIn < 0 { diff --git a/management/server/http/handlers/setup_keys/setupkeys_handler_test.go b/management/server/http/handlers/setup_keys/setupkeys_handler_test.go index b137b6dd1..a9cfd4bd3 100644 --- a/management/server/http/handlers/setup_keys/setupkeys_handler_test.go +++ b/management/server/http/handlers/setup_keys/setupkeys_handler_test.go @@ -134,6 +134,40 @@ func TestSetupKeysHandlers(t *testing.T) { expectedBody: true, expectedSetupKey: expectedNewKey, }, + { + // A one-off key is used once. Asking for more used to be accepted + // and then quietly reduced to 1. + name: "Create One-Off Setup Key With Conflicting Usage Limit", + requestType: http.MethodPost, + requestPath: "/api/setup-keys", + requestBody: bytes.NewBuffer( + []byte(fmt.Sprintf("{\"name\":\"%s\",\"type\":\"one-off\",\"expires_in\":86400,\"usage_limit\":5}", newSetupKeyName))), + expectedStatus: http.StatusUnprocessableEntity, + expectedBody: false, + }, + { + // 0 is what a caller sends when it has nothing to say about the + // usage limit, since the field is required and has no null, so it + // has to keep working. + name: "Create One-Off Setup Key Without Usage Limit", + requestType: http.MethodPost, + requestPath: "/api/setup-keys", + requestBody: bytes.NewBuffer( + []byte(fmt.Sprintf("{\"name\":\"%s\",\"type\":\"one-off\",\"expires_in\":86400,\"usage_limit\":0}", newSetupKeyName))), + expectedStatus: http.StatusOK, + expectedBody: false, + }, + { + // Only one-off keys are constrained; a reusable key means what it + // says. + name: "Create Reusable Setup Key With Usage Limit", + requestType: http.MethodPost, + requestPath: "/api/setup-keys", + requestBody: bytes.NewBuffer( + []byte(fmt.Sprintf("{\"name\":\"%s\",\"type\":\"reusable\",\"expires_in\":86400,\"usage_limit\":5}", newSetupKeyName))), + expectedStatus: http.StatusOK, + expectedBody: false, + }, { name: "Update Setup Key", requestType: http.MethodPut, diff --git a/management/server/http/testing/integration/setupkeys_handler_integration_test.go b/management/server/http/testing/integration/setupkeys_handler_integration_test.go index 0d3aaac82..21ad9d347 100644 --- a/management/server/http/testing/integration/setupkeys_handler_integration_test.go +++ b/management/server/http/testing/integration/setupkeys_handler_integration_test.go @@ -136,7 +136,10 @@ func Test_SetupKeys_Create(t *testing.T) { }, }, { - name: "Create Setup Key as on-off with more than one usage", + // The key used to be created anyway, with its usage limit quietly + // reduced to 1, so the caller was told a key they had not asked for + // was what they asked for. + name: "Create Setup Key as one-off with more than one usage", requestType: http.MethodPost, requestPath: "/api/setup-keys", requestBody: &api.CreateSetupKeyRequest{ @@ -146,23 +149,7 @@ func Test_SetupKeys_Create(t *testing.T) { Type: "one-off", UsageLimit: 3, }, - expectedStatus: http.StatusOK, - expectedResponse: &api.SetupKey{ - AutoGroups: []string{}, - Ephemeral: false, - Expires: time.Time{}, - Id: "", - Key: "", - LastUsed: time.Time{}, - Name: testing_tools.NewKeyName, - Revoked: false, - State: "valid", - Type: "one-off", - UpdatedAt: time.Now(), - UsageLimit: 1, - UsedTimes: 0, - Valid: true, - }, + expectedStatus: http.StatusUnprocessableEntity, }, { name: "Create Setup Key with expiration in the past",