callback endpoint renders more useful user-facing error messages

Co-authored-by: Joshua Casey <joshuatcasey@gmail.com>
This commit is contained in:
Ryan Richard
2024-11-21 13:01:32 -08:00
committed by Joshua Casey
parent 51ae782135
commit ecd23e86ce
2 changed files with 86 additions and 10 deletions

View File

@@ -7,9 +7,11 @@ package callback
import (
"net/http"
"net/url"
"strings"
"github.com/ory/fosite"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apiserver/pkg/audit"
"go.pinniped.dev/internal/auditevent"
"go.pinniped.dev/internal/federationdomain/downstreamsession"
@@ -152,9 +154,43 @@ func validateRequest(r *http.Request, stateDecoder, cookieDecoder oidc.Decoder)
if authcode(r) == "" {
plog.Info("code param not found")
return "", nil, httperr.New(http.StatusBadRequest,
"code param not found: check URL in browser's address bar for error parameters from upstream identity provider")
return "", nil, httperr.New(http.StatusBadRequest, errorMsgForNoCodeParam(r))
}
return encodedState, decodedState, nil
}
func errorMsgForNoCodeParam(r *http.Request) string {
msg := strings.Builder{}
msg.WriteString("code param not found\n\n")
errorParam, hasError := r.Form["error"]
errorDescParam, hasErrorDesc := r.Form["error_description"]
errorURIParam, hasErrorURI := r.Form["error_uri"]
if hasError {
msg.WriteString("error from external identity provider: ")
msg.WriteString(errorParam[0])
msg.WriteByte('\n')
}
if hasErrorDesc {
msg.WriteString("error_description from external identity provider: ")
msg.WriteString(errorDescParam[0])
msg.WriteByte('\n')
}
if hasErrorURI {
msg.WriteString("error_uri from external identity provider: ")
msg.WriteString(errorURIParam[0])
msg.WriteByte('\n')
}
if !hasError && !hasErrorDesc && !hasErrorURI {
msg.WriteString("Something went wrong with your authentication attempt at your external identity provider.\n")
}
msg.WriteByte('\n')
msg.WriteString("Pinniped AuditID: ")
msg.WriteString(audit.GetAuditIDTruncated(r.Context()))
return msg.String()
}

View File

@@ -29,6 +29,7 @@ import (
"go.pinniped.dev/internal/federationdomain/stateparam"
"go.pinniped.dev/internal/federationdomain/storage"
"go.pinniped.dev/internal/federationdomain/upstreamprovider"
"go.pinniped.dev/internal/here"
"go.pinniped.dev/internal/plog"
"go.pinniped.dev/internal/psession"
"go.pinniped.dev/internal/testutil"
@@ -1180,36 +1181,75 @@ func TestCallbackEndpoint(t *testing.T) {
},
},
{
name: "error redirect from upstream IDP audit logs the error params from the OAuth2 spec",
name: "error redirect from upstream IDP audit logs all the error params from the OAuth2 spec",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
method: http.MethodGet,
path: newRequestPath().WithState(happyOIDCState).WithoutCode().String() + "&error=some_error&error_description=some_description&error_uri=some_uri",
path: newRequestPath().WithState(happyOIDCState).WithoutCode().String() + "&error=some%20error&error_description=some%20description&error_uri=some%20uri",
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusBadRequest,
wantContentType: htmlContentType,
wantBody: "Bad Request: code param not found: check URL in browser's address bar for error parameters from upstream identity provider\n",
wantBody: here.Doc(`Bad Request: code param not found
error from external identity provider: some error
error_description from external identity provider: some description
error_uri from external identity provider: some uri
Pinniped AuditID: fake-audit-id
`),
wantAuditLogs: func(encodedStateParam stateparam.Encoded, sessionID string) []testutil.WantedAuditLog {
return []testutil.WantedAuditLog{
testutil.WantAuditLog("HTTP Request Parameters", map[string]any{
"params": map[string]any{
"state": "redacted",
"error": "some_error",
"error_description": "some_description",
"error_uri": "some_uri",
"error": "some error",
"error_description": "some description",
"error_uri": "some uri",
},
}),
}
},
},
{
name: "code param was not included on request",
name: "error redirect from upstream IDP when only some of the error params from the OAuth2 spec are included on the URL",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
method: http.MethodGet,
path: newRequestPath().WithState(happyOIDCState).WithoutCode().String() + "&error=some%20error&error_description=some%20description",
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusBadRequest,
wantContentType: htmlContentType,
wantBody: here.Doc(`Bad Request: code param not found
error from external identity provider: some error
error_description from external identity provider: some description
Pinniped AuditID: fake-audit-id
`),
wantAuditLogs: func(encodedStateParam stateparam.Encoded, sessionID string) []testutil.WantedAuditLog {
return []testutil.WantedAuditLog{
testutil.WantAuditLog("HTTP Request Parameters", map[string]any{
"params": map[string]any{
"state": "redacted",
"error": "some error",
"error_description": "some description",
},
}),
}
},
},
{
name: "code param was not included on request and there is no error param",
idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC(happyOIDCUpstream().Build()),
method: http.MethodGet,
path: newRequestPath().WithState(happyOIDCState).WithoutCode().String(),
csrfCookie: happyCSRFCookie,
wantStatus: http.StatusBadRequest,
wantContentType: htmlContentType,
wantBody: "Bad Request: code param not found: check URL in browser's address bar for error parameters from upstream identity provider\n",
wantBody: here.Doc(`Bad Request: code param not found
Something went wrong with your authentication attempt at your external identity provider.
Pinniped AuditID: fake-audit-id
`),
wantAuditLogs: func(encodedStateParam stateparam.Encoded, sessionID string) []testutil.WantedAuditLog {
return []testutil.WantedAuditLog{
testutil.WantAuditLog("HTTP Request Parameters", map[string]any{