Return HTTP 400 status for inactive OIDC auth methods

This commit modifies the OIDC authentication start method to return a 400 (Bad Request) error
if a client attempts to start the authentication flow against an OIDC auth method that is set to
inactive. This is something we noticed in our error logs in HCP Boundary that is impacting our
SLOs.

Previously this returned its own error code which is mapped to a 500 response. Tests have been updated
to catch this new scenario.

Signed-off-by: David Bond <davidsbond93@gmail.com>
pull/5048/head
David Bond 2 years ago
parent 84707c04cb
commit 3cf9611cc4
No known key found for this signature in database
GPG Key ID: A35B34F344ED7AFE

@ -178,7 +178,14 @@ func (s Service) authenticateOidcStart(ctx context.Context, req *pbs.Authenticat
}
authUrl, tokenId, err := oidc.StartAuth(ctx, s.oidcRepoFn, req.GetAuthMethodId(), opts...)
if err != nil {
switch {
case errors.Match(errors.T(errors.AuthMethodInactive), err):
return nil, handlers.ApiErrorWithCodeAndMessage(codes.FailedPrecondition, "Cannot start authentication against an inactive OIDC auth method")
case errors.Match(errors.T(errors.RecordNotFound), err):
return nil, handlers.ApiErrorWithCodeAndMessage(codes.NotFound, "Auth method %s was not found", req.GetAuthMethodId())
case errors.Match(errors.T(errors.InvalidParameter), err):
return nil, handlers.ApiErrorWithCodeAndMessage(codes.InvalidArgument, err.Error())
case err != nil:
event.WriteError(ctx, op, err, event.WithInfoMsg("error starting the oidc authentication flow"))
return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Error generating parameters for starting the OIDC flow. See the controller's log for more information.")
}

@ -1624,9 +1624,10 @@ func TestAuthenticate_OIDC_Start(t *testing.T) {
s := getSetup(t)
cases := []struct {
name string
request *pbs.AuthenticateRequest
wantErr error
name string
request *pbs.AuthenticateRequest
beforeTest func(t *testing.T)
wantErr error
}{
{
name: "no command",
@ -1657,6 +1658,57 @@ func TestAuthenticate_OIDC_Start(t *testing.T) {
AuthMethodId: s.authMethod.GetPublicId(),
},
},
{
name: "auth method does not exist",
request: &pbs.AuthenticateRequest{
Command: "start",
AuthMethodId: "amoidc_1234",
Attrs: &pbs.AuthenticateRequest_OidcStartAttributes{
OidcStartAttributes: &pbs.OidcStartAttributes{
RoundtripPayload: func() *structpb.Struct {
ret, err := structpb.NewStruct(map[string]any{
"foo": "bar",
"baz": true,
})
require.NoError(t, err)
return ret
}(),
},
},
},
wantErr: handlers.ApiErrorWithCode(codes.NotFound),
},
{
name: "auth method is inactive",
beforeTest: func(t *testing.T) {
r, err := s.oidcRepoFn()
require.NoError(t, err)
_, err = r.MakeInactive(s.ctx, s.authMethod.GetPublicId(), 2)
require.NoError(t, err)
t.Cleanup(func() {
_, err = r.MakePublic(s.ctx, s.authMethod.GetPublicId(), 3)
require.NoError(t, err)
})
},
request: &pbs.AuthenticateRequest{
Command: "start",
AuthMethodId: s.authMethod.GetPublicId(),
Attrs: &pbs.AuthenticateRequest_OidcStartAttributes{
OidcStartAttributes: &pbs.OidcStartAttributes{
RoundtripPayload: func() *structpb.Struct {
ret, err := structpb.NewStruct(map[string]any{
"foo": "bar",
"baz": true,
})
require.NoError(t, err)
return ret
}(),
},
},
},
wantErr: handlers.ApiErrorWithCode(codes.FailedPrecondition),
},
// NOTE: We can't really test bad roundtrip payload attributes because
// any type structpb lets us use in creation will be valid for JSON, and
// attempting to force with e.g. json.RawMessage causes structpb to
@ -1692,6 +1744,10 @@ func TestAuthenticate_OIDC_Start(t *testing.T) {
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if tc.beforeTest != nil {
tc.beforeTest(t)
}
assert, require := assert.New(t), require.New(t)
got, err := s.authMethodService.Authenticate(auth.DisabledAuthTestContext(s.iamRepoFn, s.org.GetPublicId()), tc.request)
if tc.wantErr != nil {

Loading…
Cancel
Save