search

go-pkgz / auth

Authenticator via oauth2, direct, email and telegram
https://go-pkgz.umputun.dev/auth/
MIT License
1.07k stars 84 forks source link

Fix registration of dev provider in Service.authMiddleware.Providers #201

Closed cyb3r4nt closed 5 months ago

cyb3r4nt commented 5 months ago

There was one minor problem related to the dev and Apple providers registration.

Now it is possible to have a configuration, where only one single dev provider is enabled.

Providers were not registered into Service.authMiddleware.Providers slice in the Service.AddDevProvider() and Service.AddAppleProvider() methods before.

This worked before, because other providers were registered at the same time, and authMiddleware.Providers slice was reassigned by other methods.

paskal commented 5 months ago

I reviewed the changes and I think they are not breaking anything.

The only thing I still do not understand is the original issue, could you please describe it in more detail, and maybe provide a code snippet which reproduces it? Ideally a test which reproduces the problem in master.

coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9349982315

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
v2/auth.go 22 24 91.67%
<!-- Total: 22 24 91.67% -->
Totals Coverage Status
Change from base Build 8627959567: -0.02%
Covered Lines: 2558
Relevant Lines: 3092
💛 - Coveralls
cyb3r4nt commented 5 months ago

The only thing I still do not understand is the original issue, could you please describe it in more detail, and maybe provide a code snippet which reproduces it? Ideally a test which reproduces the problem in master.

I was trying to use this auth lib in my dev environment, and only a single dev provider was configured there. It is possible to reproduce it with TestIntegrationProtected and commenting out all other providers in prepService() preparation method This check was failing for me during the request to some protected web endpoint. Same thing may be reproduced if remark42 is started only with option --auth.dev and without specifying any other providers.

The new and modified version of TestIntegrationProtected from this PR should cover this issue.

coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9387863023

Details

Totals Coverage Status
Change from base Build 8627959567: 0.6%
Covered Lines: 2577
Relevant Lines: 3092
💛 - Coveralls
coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 9387863020

Details

Totals Coverage Status
Change from base Build 8627959567: 0.6%
Covered Lines: 2577
Relevant Lines: 3092
💛 - Coveralls
paskal commented 5 months ago

@umputun, this one is good to merge.

@cyb3r4nt, please prepare an MR to v2, which would prohibit underscore in the provider name; that would be good if we know it breaks something.