Skip to content

Commit

Permalink
Azure - single tenant support and Fallback for missing email claim
Browse files Browse the repository at this point in the history
- Microsoft Graph API is called during authentication for retrieve an email address if the access token does not contain the `email` field
- added optional field `Tenant API` in Azure AD configuration form. This adds support for applications that are configured as single tenant
- updated package `68publishers/oauth`
  • Loading branch information
tg666 committed Feb 27, 2024
1 parent d608b78 commit 731b3e0
Show file tree
Hide file tree
Showing 13 changed files with 83 additions and 28 deletions.
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"68publishers/environment": "^1.0.1",
"68publishers/event-dispatcher-extra": "^1.1",
"68publishers/health-check": "^1.0.1",
"68publishers/oauth": "dev-main",
"68publishers/oauth": "^1.0",
"68publishers/omni": "0.1.x-dev",
"68publishers/smart-nette-component": "^1.0",
"68publishers/tracy-git-version": "^1.2",
Expand Down
16 changes: 7 additions & 9 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Application/GlobalSettings/GlobalSettings.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public static function default(): self
ApiCache::create(),
CrawlerSettings::fromValues(false, null, null, null, null),
EnvironmentSettings::createDefault(),
AzureAuthSettings::fromValues(false, null, null),
AzureAuthSettings::fromValues(false, null, null, null),
);
}

Expand Down
40 changes: 33 additions & 7 deletions src/Bridge/SixtyEightPublishers/OAuth/Azure/AzureAuthenticator.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
use SixtyEightPublishers\UserBundle\Domain\Command\CreateUserCommand;
use SixtyEightPublishers\UserBundle\Domain\ValueObject\UserId;
use SixtyEightPublishers\UserBundle\ReadModel\Query\GetUserByEmailAddressQuery;
use TheNetworg\OAuth2\Client\Provider\Azure;
use TheNetworg\OAuth2\Client\Token\AccessToken;
use Throwable;

final class AzureAuthenticator implements AuthenticatorInterface
Expand All @@ -39,7 +41,32 @@ public function authenticate(string $flowName, AuthorizationResult $authorizatio
{
$userData = $authorizationResult->resourceOwner->toArray();

if (empty($userData['email'] ?? '')) {
$username = $userData['email'] ?? '';
$firstname = $userData['given_name'] ?? '';
$surname = $userData['family_name'] ?? '';

if ('' === $username) {
try {
$client = $authorizationResult->client;
$token = $authorizationResult->accessToken;
assert($client instanceof Azure && $token instanceof AccessToken);

$data = $client->get($client->getRootMicrosoftGraphUri($token) . '/v1.0/me', $token);
$username = $data['mail'] ?? '';
$firstname = $data['givenName'] ?? $firstname;
$surname = $data['surname'] ?? $surname;
} catch (Throwable $e) {
$this->logger->error(sprintf(
'Unable to request profile for user with oid %s via %s.',
$authorizationResult->resourceOwner->getId(),
$flowName,
));

throw new AuthenticationException($e->getMessage(), 0, $e);
}
}

if ('' === $username) {
$this->logger->error(sprintf(
'Unable to login user with oid %s via %s. Missing claim for property "email".',
$authorizationResult->resourceOwner->getId(),
Expand All @@ -49,8 +76,6 @@ public function authenticate(string $flowName, AuthorizationResult $authorizatio
throw new AuthenticationException('Missing claim for property "email".');
}

$username = $userData['email'];

try {
$userView = $this->queryBus->dispatch(GetUserByEmailAddressQuery::create(
emailAddress: $username,
Expand Down Expand Up @@ -80,6 +105,8 @@ public function authenticate(string $flowName, AuthorizationResult $authorizatio
flowName: $flowName,
username: $username,
roles: $roles,
firstname: (string) $firstname,
surname: (string) $surname,
);
} else {
$userId = $userView->id->toString();
Expand Down Expand Up @@ -111,18 +138,17 @@ public function authenticate(string $flowName, AuthorizationResult $authorizatio
/**
* @param array<int, string> $roles
*/
private function createUser(ResourceOwnerInterface $resourceOwner, string $flowName, string $username, array $roles): string
private function createUser(ResourceOwnerInterface $resourceOwner, string $flowName, string $username, array $roles, string $firstname, string $surname): string
{
$userId = UserId::new()->toString();
$userData = $resourceOwner->toArray();

try {
$command = CreateUserCommand::create(
username: $username,
password: null,
emailAddress: $username,
firstname: $userData['given_name'] ?? '',
surname: $userData['family_name'] ?? '',
firstname: $firstname,
surname: $surname,
roles: $roles,
userId: $userId,
);
Expand Down
1 change: 1 addition & 0 deletions src/Bridge/SixtyEightPublishers/OAuth/Azure/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public function __construct(GlobalSettingsInterface $globalSettings)
options: [
AzureAuthorizator::OptClientId => $azureAuthSetting->clientId(),
AzureAuthorizator::OptClientSecret => $azureAuthSetting->clientSecret(),
AzureAuthorizator::OptTenantId => $azureAuthSetting->tenantId(),
],
);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@

final class PutAzureAuthSettingsCommand extends AbstractCommand
{
public static function create(bool $enabled, ?string $clientId, ?string $clientSecret): self
public static function create(bool $enabled, ?string $clientId, ?string $clientSecret, ?string $tenantId): self
{
return self::fromParameters([
'enabled' => $enabled,
'client_id' => $clientId,
'client_secret' => $clientSecret,
'tenant_id' => $tenantId,
]);
}

Expand All @@ -31,4 +32,9 @@ public function clientSecret(): ?string
{
return $this->getParam('client_secret');
}

public function tenantId(): ?string
{
return $this->getParam('tenant_id');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ public function __invoke(PutAzureAuthSettingsCommand $command): void
}

$globalSettings->updateAzureAuthSettings(AzureAuthSettings::fromValues(
$command->enabled(),
$command->clientId(),
$command->clientSecret(),
enabled: $command->enabled(),
clientId: $command->clientId(),
clientSecret: $command->clientSecret(),
tenantId: $command->tenantId(),
));

$this->globalSettingsRepository->save($globalSettings);
Expand Down
2 changes: 1 addition & 1 deletion src/Domain/GlobalSettings/GlobalSettings.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ protected function whenGlobalSettingsCreated(GlobalSettingsCreated $event): void
$this->apiCache = ApiCache::create();
$this->crawlerSettings = CrawlerSettings::fromValues(false, null, null, null, null);
$this->environmentSettings = EnvironmentSettings::createDefault();
$this->azureAuthSettings = AzureAuthSettings::fromValues(false, null, null);
$this->azureAuthSettings = AzureAuthSettings::fromValues(false, null, null, null);
}

protected function whenLocalizationSettingsChanged(LocalizationSettingsChanged $event): void
Expand Down
7 changes: 7 additions & 0 deletions src/Domain/GlobalSettings/ValueObject/AzureAuthSettings.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ public static function fromValues(
bool $enabled,
?string $clientId,
?string $clientSecret,
?string $tenantId,
): self {
return self::fromArray([
'enabled' => $enabled,
'client_id' => $clientId,
'client_secret' => $clientSecret,
'tenant_id' => $tenantId,
]);
}

Expand All @@ -34,4 +36,9 @@ public function clientSecret(): ?string
{
return $this->get('client_secret');
}

public function tenantId(): ?string
{
return $this->get('tenant_id');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ protected function createComponentForm(): Form
$enabledField->addCondition($form::EQUAL, true)
->toggle('#' . $this->getUniqueId() . '-client_id-container')
->toggle('#' . $this->getUniqueId() . '-client_secret-container')
->toggle('#' . $this->getUniqueId() . '-tenant_id-container')
->toggle('#' . $this->getUniqueId() . '-callback_uri-container');

$form->addText('client_id', 'client_id.field')
Expand All @@ -53,6 +54,10 @@ protected function createComponentForm(): Form
->addConditionOn($enabledField, $form::EQUAL, true)
->setRequired('client_secret.required');

$form->addText('tenant_id', 'tenant_id.field')
->setOption('id', $this->getUniqueId() . '-tenant_id-container')
->setOption('description', 'tenant_id.description');

$form->addText('callback_uri', 'callback_uri.field')
->setDisabled()
->setOmitted()
Expand All @@ -71,6 +76,7 @@ protected function createComponentForm(): Form
'enabled' => $defaults->enabled(),
'client_id' => (string) $defaults->clientId(),
'client_secret' => (string) $defaults->clientSecret(),
'tenant_id' => (string) $defaults->tenantId(),
]);

$form->onSuccess[] = function (Form $form): void {
Expand All @@ -84,9 +90,10 @@ private function saveGlobalSettings(Form $form): void
{
$values = $form->getValues();
$command = PutAzureAuthSettingsCommand::create(
$values->enabled,
$values->client_id ?: null,
$values->client_secret ?: null,
enabled: $values->enabled,
clientId: $values->client_id ?: null,
clientSecret: $values->client_secret ?: null,
tenantId: $values->tenant_id ?: null,
);

try {
Expand Down
3 changes: 2 additions & 1 deletion src/Web/FrontModule/Presenter/OAuthPresenter.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@

namespace App\Web\FrontModule\Presenter;

use App\Web\Ui\Presenter;
use Psr\Log\LoggerInterface;
use SixtyEightPublishers\FlashMessageBundle\Domain\FlashMessage;
use SixtyEightPublishers\OAuth\Bridge\Nette\Application\OAuthPresenterTrait;
use SixtyEightPublishers\OAuth\Exception\OAuthExceptionInterface;

final class OAuthPresenter extends FrontPresenter
final class OAuthPresenter extends Presenter
{
use OAuthPresenterTrait;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ client_secret:
field: Tajný kód klienta
required: Vyplňte prosím tajný kód klienta.

tenant_id:
field: 'ID adresáře (tenanta)'
description: 'Vyplňte pouze pokud je v nastavení aplikace na webu Azure nastaveno "Podporované typy účtu" na možnost "Jen účty v tomto adresáři organizace" - jeden tenant. Jinak ponechte prázdné.'

callback_uri:
field: Callback URI
description: 'V nastavení aplikace na webu Azure nastavte jako "Identifikátor URI pro přesměrování".'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ enabled:
field: Enabled

client_id:
field: 'Application ID (client)'
field: 'Application (client) ID'
required: Please enter a application ID.

client_secret:
field: Client secret code
required: Please enter a client secret code.

tenant_id:
field: 'Directory (tenant) ID'
description: 'Fill only if the "Supported account types" option in the application settings on the Azure site is set to "Accounts in this organizational directory only" - single tenant. Otherwise, leave blank.'

callback_uri:
field: Callback URI
description: 'In the application settings on the Azure site, set as the "Redirect URI".'
Expand Down

0 comments on commit 731b3e0

Please sign in to comment.