Skip to content

Conversation

zyAmo
Copy link
Contributor

@zyAmo zyAmo commented Jul 26, 2024

original description at #702

This PR extends the existing IAuthentication interface's Callback() function to include the *http.Request parameter.

This enhancement allows plugins implementing this interface to access the HTTP request directly, and thereby enabling them to retrieve additional information, such as cookies, via req.Cookie("my-cookie"). Additionally, it allows for request parsing to be handled by the plugin itself if desired.
Processing cookies during the Callback() can be highly beneficial for various login flows.

The PR is structured into two commits:

  1. first commit
    • extends the interface and updates existing plugins accordingly

Since this addition only involves adding a new parameter to the function signature, which is currently ignored in all existing plugins, it can be considered a minor change with essentially no impact on existing authentication plugins, although their signatures have to be updated, as I have done in this PR.

  1. second commit:
    • removes formData from the function signature to avoid redundancy in passing both the request and the parsed body
    • moves the existing parsing code to a utility function, which can be called from within the plugin, ensuring full compatibility with existing plugins

This introduces a minor change in error handling: Parsing errors from req.ParseForm() are now wrapped as NewError(err.Error(), 400) and handled by the error handling in session.go after the call to IAuthentication.Callback(), resulting in a slightly changed redirect behavior:

  • before: redirect to /?error=Not%20Valid&trace=parsing%20body%20-<error message>
  • after: redirect to /?error=Not%20Allowed&trace=redirect%20request%20failed%20-<error message>

If the new redirect behavior is undesirable, it can be adjusted to handle the error differently, although currently, all non-ErrAuthenticationFailed errors inside the Callback() are treated this way. Alternatively, the second commit can be left out, ignoring the redundancy in favor of simplicity. I'm happy for feedback.


Thank you for considering this change to improve the IAuthentication interface :)

currently waiting for frontend rewrite

@mickael-kerjean mickael-kerjean force-pushed the master branch 6 times, most recently from b95fa04 to 11c4a74 Compare September 2, 2025 13:52
@mickael-kerjean mickael-kerjean force-pushed the master branch 5 times, most recently from 9757ff1 to 7fa253f Compare September 11, 2025 05:00
@mickael-kerjean mickael-kerjean force-pushed the master branch 2 times, most recently from 5e9f59a to b9f2ee9 Compare September 23, 2025 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant