-
-
Notifications
You must be signed in to change notification settings - Fork 134
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
Add support for SSO authentication #1831
Conversation
41fc3de
to
fe93707
Compare
fe93707
to
a45ddbc
Compare
@jderusse I can try this out sometime in the new few hours. Thank you! |
@jderusse took me a couple of hours, and a sleep, to realize that I had a configuration bug when testing this. Now that I have resolved that, I am happy to report that this PR works perfectly. 🎉 |
|
||
return $this->getCredentialsFromSsoSession($profilesData, $profileData, $profile); | ||
} | ||
|
||
if (isset($profileData[IniFileLoader::KEY_SSO_START_URL])) { | ||
if (class_exists(SsoClient::class)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change the branches without inverting the condition
@@ -93,14 +93,24 @@ private function getCredentialsFromProfile(array $profilesData, string $profile, | |||
return $this->getCredentialsFromRole($profilesData, $profileData, $profile, $circularCollector); | |||
} | |||
|
|||
if (isset($profileData[IniFileLoader::KEY_SSO_SESSION])) { | |||
if (!class_exists(SsoClient::class)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this check for SsoOidcClient
instead ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
already addressed by #1833
512, | ||
\JSON_BIGINT_AS_STRING | (\PHP_VERSION_ID >= 70300 ? \JSON_THROW_ON_ERROR : 0) | ||
); | ||
} catch (\JsonException $e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you also need the error handling for PHP 7.2 as it won't throw an error there.
Fix #1581