feat(rp): Add UnauthorizedHandler

[jkroepke]

Any callback error which results into an unauthorized status just print as plain text to the user.

I would like to enrich the user experience and offer an HTML styled error message.

My first idea was to re-use the existing errorHandler, but it always returns http.StatusInternalServerError. The best possible solution would be an errorHandler which has the preferred http status as function input. However this breaks the function signature.

Definition of Ready

• I am happy with the code
• Short description of the feature/issue is added in the pr description
• PR is linked to the corresponding user story
• Acceptance criteria are met
• All open todos and follow ups are defined in a new ticket and justified
• Deviations from the acceptance criteria and design are agreed with the PO and documented.
• No debug or dead code
• My code has no repetitions
• Critical parts are tested automatically
• Where possible E2E tests are implemented
• Documentation/examples are up-to-date
• All non-functional requirements are met
• Functionality of the acceptance criteria is checked manually on the dev system.

[muhlemmer]

Adding a method to the interface would be a breaking change. For the proposed implementation we don't need to expose the method, right?

[jkroepke]

Correct, we do not need to expose the method. however CodeExchangeHandler is using the interface as input. Without declare the function on the interface, the method is not available.

oidc/pkg/client/rp/relying_party.go

Line 448 in 2b35eeb

func CodeExchangeHandler[C oidc.IDClaims](callback CodeExchangeCallback[C], rp RelyingParty, urlParam ...URLParamOpt) http.HandlerFunc {

As I mention, I would prefer to re-use the existing rp.ErrorHandler, but it does not fit in this case here.

[codecov]

Codecov Report

Attention: 26 lines in your changes are missing coverage. Please review.

Comparison is base (4d85375) 60.12% compared to head (763e05f) 59.98%.

Files	Patch %	Lines
pkg/client/rp/relying_party.go	0.00%	26 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #503      +/-   ##
==========================================
- Coverage   60.12%   59.98%   -0.14%     
==========================================
  Files          80       80              
  Lines        6962     6978      +16     
==========================================
  Hits         4186     4186              
- Misses       2480     2496      +16     
  Partials      296      296              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jkroepke]

@muhlemmer do you an alternative solution here? If I had the choice, I would think about an breaking change which modifies the function signature of the existing ErrorHandler by accepting the http status as input parameter and use the ErrorHandler on all places. I think this is more convenient as having two functions.

[muhlemmer]

For this PR to the main branch you could use an optional interface:

type HasUnauthorizedHandler interface {
	// UnauthorizedHandler returns the handler used for unauthorized errors
	UnauthorizedHandler() UnauthorizedHandler
}

Then use type-assertion in a helper function to use either the new or old behavior:

func unauthorizedError(w http.ResponseWriter, r *http.Request, desc string, state string, rp RelyingParty) {
	if rp, ok := rp.(HasUnauthorizedHandler); ok {
		rp.UnauthorizedHandler()(w, r, desc, state)
		return
	}
	http.Error(w, desc, http.StatusUnauthorized)
}

And call that function where required:

if err := trySetStateCookie(w, state, rp); err != nil {
    unauthorizedError(w, r, "failed to create state cookie: "+err.Error(), state, rp)
    return
}

After we merged this we can create a new PR against the next branch and do the breaking changes for the v4 release.

[jkroepke]

Sounds interesting, I was not aware of pattern. Thanks for bringing this in.

[github-actions]

🎉 This PR is included in version 3.10.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀