auth icon indicating copy to clipboard operation
auth copied to clipboard

Improve provider name handling

Open cyb3r4nt opened this issue 1 year ago • 2 comments

The problem occurs when the provider name contains underscore characters _. If provider name is like provider_prod and full claims.User.ID in the JWT token looks like provider_prod_user1, then Authenticator.isProviderAllowed() check fails and provider with such name cannot be used. This was initially discovered in https://github.com/go-pkgz/auth/pull/201#discussion_r1624361335.

It might be better to add an explicit provider name into the JWT token claims, and avoid parsing already serialized string back to tokens.

Provider name passed into Service.AddProvider() also becomes a part of https://host:port/auth/provider_prod/login URL, and therefore it requires special handling. One solution is to url-encode it, but then it will be still possible to use names containing spaces or special characters (by accident or with purpose). Another solution is to forbid all provider names which require url-encoding. It might be better to forbid empty names as well. _ underscore has been mentioned in the README examples for some time now, i am not sure about it. But those names may be even more strict and contain only ASCII alphanumeric symbols. What do you think?

It is not possible to return errors from Service.AddProvider(), therefore invalid providers are just ignored and ERROR level message is logged.

cyb3r4nt avatar Aug 29 '24 21:08 cyb3r4nt

golangci-lint fixes are in https://github.com/go-pkgz/auth/pull/214

cyb3r4nt avatar Aug 30 '24 13:08 cyb3r4nt

Pull Request Test Coverage Report for Build 14035262176

Details

  • 64 of 64 (100.0%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 83.582%

Totals Coverage Status
Change from base Build 14023741507: 0.2%
Covered Lines: 2688
Relevant Lines: 3216

💛 - Coveralls

coveralls avatar Sep 02 '24 06:09 coveralls

Rebased on top of master, without altering anything. @umputun could you please take a look?

paskal avatar Jan 02 '25 20:01 paskal