Refine user header code #816
Conversation
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #816 +/- ##
==========================================
- Coverage 65.83% 65.72% -0.11%
==========================================
Files 53 53
Lines 7671 7691 +20
==========================================
+ Hits 5050 5055 +5
- Misses 1781 1794 +13
- Partials 840 842 +2
☔ View full report in Codecov by Sentry. |
| } | ||
| if s.config.AuthUserHeader != "" && s.config.BehindProxy { | ||
| username := r.Header.Get(s.config.AuthUserHeader) // Do not allow a query param, only a header! | ||
| if username != "" { |
There was a problem hiding this comment.
Up to you, but shouldn't this be located inside authenticateViaUserDefinedHeader(), to keep the abstraction layer more consistent (i.e. for the auth header authn, the header is alos read in authenticateViaUserDefinedHeader(), not in the caller function)?
There was a problem hiding this comment.
I did realize, that if the username is not provided, you will fall back to the default authorization, so ignore my comment.
|
I've added the tests to the brnch of the original PR, together with your changes in #812 |
Refined #812