Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GitHub OAuth2 for accounts without email #233

Merged
merged 5 commits into from
Feb 12, 2024

Conversation

MarcHagen
Copy link
Contributor

So maybe I'm one of the few people using the GitHub OAuth.
But it not working correctly.


It throws an error:

Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge::__construct(): Argument #1 ($userIdentifier) must be of type string, null given, called in /var/www/packagist/src/Integrations/Security/OAuth2Authenticator.php on line 79

This is because $event->getPassport()?->getUser()->getUserIdentifier() returns null. which is not accepted by logWebLogin


So that fixes one issue, but yes, another one arose. (like always...)

Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge::__construct(): Argument #1 ($userIdentifier) must be of type string, null given, called in /var/www/packagist/src/Integrations/Security/OAuth2Authenticator.php on line 79

So appears $response['email'] is NULL on my account,
this is because I've enabled the GitHub option: Keep my email addresses private

So in this case, we can use the username as user_identifier.
Further down in the public function createUser it breaks hard on the same issue with email as a database constrained.
I've copied the $email part from BaseIntegrationTrait.


Also added .docker to the gitignore and renamed the Phpunit folder to PHPUnitOverride.
As phpunit the file and Phpunit the folder will not work on windows due to naming stuff and ... windows.

Argument vtsykun#2 ($user) must be of type Symfony\Component\Security\Core\User\UserInterface|string, null given
@MarcHagen MarcHagen requested a review from vtsykun as a code owner February 12, 2024 16:55
@vtsykun vtsykun merged commit e3e1830 into vtsykun:master Feb 12, 2024
4 checks passed
@MarcHagen MarcHagen deleted the fix-oauth2 branch February 12, 2024 20:40
@vtsykun
Copy link
Owner

vtsykun commented Feb 12, 2024

Thanks!

@REBELinBLUE
Copy link

REBELinBLUE commented May 15, 2024

@vtsykun Any chance of a new docker image release with this fix included anytime soon?

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.

3 participants