diff --git a/api/components/OAuth2/Component.php b/api/components/OAuth2/Component.php index 178adce..0b77dee 100644 --- a/api/components/OAuth2/Component.php +++ b/api/components/OAuth2/Component.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace api\components\OAuth2; use api\components\OAuth2\Keys\EmptyKey; -use api\components\OAuth2\Repositories; use DateInterval; use League\OAuth2\Server\AuthorizationServer; use League\OAuth2\Server\Grant; diff --git a/api/components/OAuth2/Repositories/AuthCodeStorage.php b/api/components/OAuth2/Repositories/AuthCodeStorage.php deleted file mode 100644 index a083064..0000000 --- a/api/components/OAuth2/Repositories/AuthCodeStorage.php +++ /dev/null @@ -1,72 +0,0 @@ -dataTable, $code))->getValue()); - if ($result === null) { - return null; - } - - $entity = new AuthCodeEntity($this->server); - $entity->setId($result['id']); - $entity->setExpireTime($result['expire_time']); - $entity->setSessionId($result['session_id']); - $entity->setRedirectUri($result['client_redirect_uri']); - - return $entity; - } - - public function create($token, $expireTime, $sessionId, $redirectUri) { - $payload = Json::encode([ - 'id' => $token, - 'expire_time' => $expireTime, - 'session_id' => $sessionId, - 'client_redirect_uri' => $redirectUri, - ]); - - $this->key($token)->setValue($payload)->expireAt($expireTime); - } - - public function getScopes(OriginalAuthCodeEntity $token) { - $scopes = $this->scopes($token->getId()); - $scopesEntities = []; - foreach ($scopes as $scope) { - if ($this->server->getScopeStorage()->get($scope) !== null) { - $scopesEntities[] = (new ScopeEntity($this->server))->hydrate(['id' => $scope]); - } - } - - return $scopesEntities; - } - - public function associateScope(OriginalAuthCodeEntity $token, ScopeEntity $scope) { - $this->scopes($token->getId())->add($scope->getId())->expireAt($token->getExpireTime()); - } - - public function delete(OriginalAuthCodeEntity $token) { - $this->key($token->getId())->delete(); - $this->scopes($token->getId())->delete(); - } - - private function key(string $token): Key { - return new Key($this->dataTable, $token); - } - - private function scopes(string $token): Set { - return new Set($this->dataTable, $token, 'scopes'); - } - -} diff --git a/api/components/OAuth2/Repositories/ClientRepository.php b/api/components/OAuth2/Repositories/ClientRepository.php index c44b496..05d4231 100644 --- a/api/components/OAuth2/Repositories/ClientRepository.php +++ b/api/components/OAuth2/Repositories/ClientRepository.php @@ -29,8 +29,6 @@ class ClientRepository implements ClientRepositoryInterface { return false; } - // TODO: there is missing behavior of checking redirectUri. Is it now bundled into grant? - return true; } diff --git a/api/components/OAuth2/Repositories/SessionStorage.php b/api/components/OAuth2/Repositories/SessionStorage.php deleted file mode 100644 index c598db2..0000000 --- a/api/components/OAuth2/Repositories/SessionStorage.php +++ /dev/null @@ -1,97 +0,0 @@ -hydrate($this->getSessionModel($sessionId)); - } - - public function getByAccessToken(OriginalAccessTokenEntity $accessToken) { - throw new ErrorException('This method is not implemented and should not be used'); - } - - public function getByAuthCode(OriginalAuthCodeEntity $authCode) { - if (!$authCode instanceof AuthCodeEntity) { - throw new ErrorException('This module assumes that $authCode typeof ' . AuthCodeEntity::class); - } - - return $this->getById($authCode->getSessionId()); - } - - public function getScopes(OriginalSessionEntity $session) { - $result = []; - foreach ($this->getSessionModel($session->getId())->getScopes() as $scope) { - if ($this->server->getScopeStorage()->get($scope) !== null) { - $result[] = (new ScopeEntity($this->server))->hydrate(['id' => $scope]); - } - } - - return $result; - } - - public function create($ownerType, $ownerId, $clientId, $clientRedirectUri = null) { - $sessionId = OauthSession::find() - ->select('id') - ->andWhere([ - 'client_id' => $clientId, - 'owner_type' => $ownerType, - 'owner_id' => (string)$ownerId, // Casts as a string to make the indexes work, because the varchar field - ])->scalar(); - - if ($sessionId === false) { - $model = new OauthSession(); - $model->client_id = $clientId; - $model->owner_type = $ownerType; - $model->owner_id = $ownerId; - $model->client_redirect_uri = $clientRedirectUri; - - if (!$model->save()) { - throw new Exception('Cannot save ' . OauthSession::class . ' model.'); - } - - $sessionId = $model->id; - } - - return $sessionId; - } - - public function associateScope(OriginalSessionEntity $session, ScopeEntity $scope) { - $this->getSessionModel($session->getId())->getScopes()->add($scope->getId()); - } - - private function getSessionModel(string $sessionId): OauthSession { - $session = OauthSession::findOne($sessionId); - if ($session === null) { - throw new ErrorException('Cannot find oauth session'); - } - - return $session; - } - - private function hydrate(OauthSession $sessionModel) { - $entity = new SessionEntity($this->server); - $entity->setId($sessionModel->id); - $entity->setClientId($sessionModel->client_id); - $entity->setOwner($sessionModel->owner_type, $sessionModel->owner_id); - - return $entity; - } - -} diff --git a/api/components/OAuth2/RequestTypes/AuthorizationRequestProxy.php b/api/components/OAuth2/RequestTypes/AuthorizationRequestProxy.php new file mode 100644 index 0000000..0d07bc1 --- /dev/null +++ b/api/components/OAuth2/RequestTypes/AuthorizationRequestProxy.php @@ -0,0 +1,97 @@ +authorizationRequest = $authorizationRequest; + } + + public function getOriginalAuthorizationRequest(): AuthorizationRequest { + return $this->authorizationRequest; + } + + public function getGrantTypeId(): string { + return $this->authorizationRequest->getGrantTypeId(); + } + + public function setGrantTypeId($grantTypeId): void { + $this->authorizationRequest->setGrantTypeId($grantTypeId); + } + + public function getClient(): ClientEntityInterface { + return $this->authorizationRequest->getClient(); + } + + public function setClient(ClientEntityInterface $client): void { + $this->authorizationRequest->setClient($client); + } + + public function getUser(): UserEntityInterface { + return $this->authorizationRequest->getUser(); + } + + public function setUser(UserEntityInterface $user): void { + $this->authorizationRequest->setUser($user); + } + + public function getScopes(): array { + return $this->authorizationRequest->getScopes(); + } + + public function setScopes(array $scopes): void { + $this->authorizationRequest->setScopes($scopes); + } + + public function isAuthorizationApproved(): bool { + return $this->authorizationRequest->isAuthorizationApproved(); + } + + public function setAuthorizationApproved($authorizationApproved): void { + $this->authorizationRequest->setAuthorizationApproved($authorizationApproved); + } + + public function getRedirectUri(): ?string { + return $this->authorizationRequest->getRedirectUri(); + } + + public function setRedirectUri($redirectUri): void { + $this->authorizationRequest->setRedirectUri($redirectUri); + } + + public function getState(): ?string { + return $this->authorizationRequest->getState(); + } + + public function setState($state): void { + $this->authorizationRequest->setState($state); + } + + public function getCodeChallenge(): string { + return $this->authorizationRequest->getCodeChallenge(); + } + + public function setCodeChallenge($codeChallenge): void { + $this->authorizationRequest->setCodeChallenge($codeChallenge); + } + + public function getCodeChallengeMethod(): string { + return $this->authorizationRequest->getCodeChallengeMethod(); + } + + public function setCodeChallengeMethod($codeChallengeMethod): void { + $this->authorizationRequest->setCodeChallengeMethod($codeChallengeMethod); + } + +} diff --git a/api/exceptions/ThisShouldNotHappenException.php b/api/exceptions/ThisShouldNotHappenException.php index 0c18aeb..bd6921c 100644 --- a/api/exceptions/ThisShouldNotHappenException.php +++ b/api/exceptions/ThisShouldNotHappenException.php @@ -7,6 +7,8 @@ namespace api\exceptions; * The exception can be used for cases where the outcome doesn't seem to be expected, * but can theoretically happen. The goal is to capture these areas and refine the logic * if such situations do occur. + * + * @deprecated use \Webmozart\Assert\Assert to ensure, that action has been successfully performed */ class ThisShouldNotHappenException extends Exception { diff --git a/api/modules/oauth/models/OauthProcess.php b/api/modules/oauth/models/OauthProcess.php index 984c72e..43f83cc 100644 --- a/api/modules/oauth/models/OauthProcess.php +++ b/api/modules/oauth/models/OauthProcess.php @@ -7,12 +7,15 @@ use api\components\OAuth2\Entities\UserEntity; use api\rbac\Permissions as P; use common\models\Account; use common\models\OauthClient; +use common\models\OauthSession; use GuzzleHttp\Psr7\Response; use GuzzleHttp\Psr7\ServerRequest; use League\OAuth2\Server\AuthorizationServer; +use League\OAuth2\Server\Entities\ScopeEntityInterface; use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\RequestTypes\AuthorizationRequest; use Psr\Http\Message\ServerRequestInterface; +use Webmozart\Assert\Assert; use Yii; class OauthProcess { @@ -57,7 +60,7 @@ class OauthProcess { $clientModel = $this->findClient($client->getIdentifier()); $response = $this->buildSuccessResponse($request, $clientModel, $authRequest->getScopes()); } catch (OAuthServerException $e) { - $response = $this->buildErrorResponse($e); + $response = $this->buildCompleteErrorResponse($e); } return $response; @@ -90,29 +93,31 @@ class OauthProcess { $authRequest = $this->server->validateAuthorizationRequest($request); /** @var Account $account */ $account = Yii::$app->user->identity->getAccount(); - /** @var OauthClient $clientModel */ - $clientModel = $this->findClient($authRequest->getClient()->getIdentifier()); + /** @var OauthClient $client */ + $client = $this->findClient($authRequest->getClient()->getIdentifier()); - if (!$this->canAutoApprove($account, $clientModel, $authRequest)) { + $approved = $this->canAutoApprove($account, $client, $authRequest); + if (!$approved) { Yii::$app->statsd->inc('oauth.complete.approve_required'); - $accept = ((array)$request->getParsedBody())['accept'] ?? null; - if ($accept === null) { + $acceptParam = ((array)$request->getParsedBody())['accept'] ?? null; + if ($acceptParam === null) { throw $this->createAcceptRequiredException(); } - if (!in_array($accept, [1, '1', true, 'true'], true)) { - throw OAuthServerException::accessDenied(null, $authRequest->getRedirectUri()); + $approved = in_array($acceptParam, [1, '1', true, 'true'], true); + if ($approved) { + $this->storeOauthSession($account, $client, $authRequest); } } $authRequest->setUser(new UserEntity($account->id)); - $authRequest->setAuthorizationApproved(true); - $responseObj = $this->server->completeAuthorizationRequest($authRequest, new Response(200)); + $authRequest->setAuthorizationApproved($approved); + $response = $this->server->completeAuthorizationRequest($authRequest, new Response(200)); - $response = [ + $result = [ 'success' => true, - 'redirectUri' => $responseObj->getHeader('Location'), // TODO: ensure that this is correct type and behavior + 'redirectUri' => $response->getHeaderLine('Location'), ]; Yii::$app->statsd->inc('oauth.complete.success'); @@ -121,10 +126,10 @@ class OauthProcess { Yii::$app->statsd->inc('oauth.complete.fail'); } - $response = $this->buildErrorResponse($e); + $result = $this->buildCompleteErrorResponse($e); } - return $response; + return $result; } /** @@ -168,10 +173,7 @@ class OauthProcess { Yii::$app->statsd->inc("oauth.issueToken_{$grantType}.fail"); Yii::$app->response->statusCode = $e->getHttpStatusCode(); - $response = [ - 'error' => $e->getErrorType(), - 'message' => $e->getMessage(), // TODO: use hint field? - ]; + $response = $this->buildIssueErrorResponse($e); } return $response; @@ -196,22 +198,31 @@ class OauthProcess { return true; } - /** @var \common\models\OauthSession|null $session */ - $session = $account->getOauthSessions()->andWhere(['client_id' => $client->id])->one(); - if ($session !== null) { - $existScopes = $session->getScopes()->members(); - if (empty(array_diff(array_keys($request->getScopes()), $existScopes))) { - return true; - } + $session = $this->findOauthSession($account, $client); + if ($session === null) { + return false; } - return false; + return empty(array_diff($this->getScopesList($request), $session->getScopes())); + } + + private function storeOauthSession(Account $account, OauthClient $client, AuthorizationRequest $request): void { + $session = $this->findOauthSession($account, $client); + if ($session === null) { + $session = new OauthSession(); + $session->account_id = $account->id; + $session->client_id = $client->id; + } + + $session->scopes = array_unique(array_merge($session->getScopes(), $this->getScopesList($request))); + + Assert::true($session->save()); } /** * @param ServerRequestInterface $request * @param OauthClient $client - * @param \League\OAuth2\Server\Entities\ScopeEntityInterface[] $scopes + * @param ScopeEntityInterface[] $scopes * * @return array */ @@ -238,7 +249,7 @@ class OauthProcess { } /** - * @param \League\OAuth2\Server\Entities\ScopeEntityInterface[] $scopes + * @param ScopeEntityInterface[] $scopes * @return array */ private function buildScopesArray(array $scopes): array { @@ -250,7 +261,7 @@ class OauthProcess { return $result; } - private function buildErrorResponse(OAuthServerException $e): array { + private function buildCompleteErrorResponse(OAuthServerException $e): array { $hint = $e->getPayload()['hint'] ?? ''; if (preg_match('/the `(\w+)` scope/', $hint, $matches)) { $parameter = $matches[1]; @@ -274,6 +285,35 @@ class OauthProcess { return $response; } + /** + * Raw error messages aren't very informative for the end user, as they don't contain + * information about the parameter that caused the error. + * This method is intended to build a more understandable description. + * + * Part of the existing texts is a legacy from the previous implementation. + * + * @param OAuthServerException $e + * @return array + */ + private function buildIssueErrorResponse(OAuthServerException $e): array { + $errorType = $e->getErrorType(); + $message = $e->getMessage(); + $hint = $e->getHint(); + switch ($hint) { + case 'Invalid redirect URI': + $errorType = 'invalid_client'; + $message = 'Client authentication failed.'; + break; + case 'Cannot decrypt the authorization code': + $message .= ' Check the "code" parameter.'; + } + + return [ + 'error' => $errorType, + 'message' => $message, + ]; + } + private function getRequest(): ServerRequestInterface { return ServerRequest::fromGlobals(); } @@ -287,4 +327,16 @@ class OauthProcess { ); } + private function getScopesList(AuthorizationRequest $request): array { + // TODO: replace with an arrow function in PHP 7.4 + return array_map(function(ScopeEntityInterface $scope): string { + return $scope->getIdentifier(); + }, $request->getScopes()); + } + + private function findOauthSession(Account $account, OauthClient $client): ?OauthSession { + /** @noinspection PhpIncompatibleReturnTypeInspection */ + return $account->getOauthSessions()->andWhere(['client_id' => $client->id])->one(); + } + } diff --git a/api/tests/functional/_steps/OauthSteps.php b/api/tests/functional/_steps/OauthSteps.php index e12d5f2..5c0559d 100644 --- a/api/tests/functional/_steps/OauthSteps.php +++ b/api/tests/functional/_steps/OauthSteps.php @@ -9,31 +9,30 @@ use api\tests\FunctionalTester; class OauthSteps extends FunctionalTester { - public function getAuthCode(array $permissions = []): string { + public function obtainAuthCode(array $permissions = []): string { $this->amAuthenticated(); - $route = new OauthRoute($this); - $route->complete([ + $this->sendPOST('/api/oauth2/v1/complete?' . http_build_query([ 'client_id' => 'ely', 'redirect_uri' => 'http://ely.by', 'response_type' => 'code', - 'scope' => implode(',', $permissions), - ], ['accept' => true]); + 'scope' => implode(' ', $permissions), + ]), ['accept' => true]); $this->canSeeResponseJsonMatchesJsonPath('$.redirectUri'); - $response = json_decode($this->grabResponse(), true); - preg_match('/code=([\w-]+)/', $response['redirectUri'], $matches); + [$redirectUri] = $this->grabDataFromResponseByJsonPath('$.redirectUri'); + preg_match('/code=([\w-]+)/', $redirectUri, $matches); return $matches[1]; } public function getAccessToken(array $permissions = []): string { - $authCode = $this->getAuthCode($permissions); + $authCode = $this->obtainAuthCode($permissions); $response = $this->issueToken($authCode); return $response['access_token']; } public function getRefreshToken(array $permissions = []): string { - $authCode = $this->getAuthCode(array_merge([S::OFFLINE_ACCESS], $permissions)); + $authCode = $this->obtainAuthCode(array_merge([S::OFFLINE_ACCESS], $permissions)); $response = $this->issueToken($authCode); return $response['refresh_token']; diff --git a/api/tests/functional/oauth/CreateClientCest.php b/api/tests/functional/dev/applications/CreateClientCest.php similarity index 98% rename from api/tests/functional/oauth/CreateClientCest.php rename to api/tests/functional/dev/applications/CreateClientCest.php index 2eab1a2..a5523a4 100644 --- a/api/tests/functional/oauth/CreateClientCest.php +++ b/api/tests/functional/dev/applications/CreateClientCest.php @@ -1,5 +1,7 @@ route = new OauthRoute($I); + public function successfullyIssueToken(OauthSteps $I) { + $I->wantTo('complete oauth flow and obtain access_token'); + $authCode = $I->obtainAuthCode(); + $I->sendPOST('/api/oauth2/v1/token', [ + 'grant_type' => 'authorization_code', + 'code' => $authCode, + 'client_id' => 'ely', + 'client_secret' => 'ZuM1vGchJz-9_UZ5HC3H3Z9Hg5PzdbkM', + 'redirect_uri' => 'http://ely.by', + ]); + $I->canSeeResponseCodeIs(200); + $I->canSeeResponseContainsJson([ + 'token_type' => 'Bearer', + ]); + $I->canSeeResponseJsonMatchesJsonPath('$.access_token'); + $I->canSeeResponseJsonMatchesJsonPath('$.expires_in'); + $I->cantSeeResponseJsonMatchesJsonPath('$.refresh_token'); } - public function testIssueTokenWithWrongArgs(OauthSteps $I) { - $I->wantTo('check behavior on on request without any credentials'); - $this->route->issueToken(); + public function successfullyIssueOfflineToken(OauthSteps $I) { + $I->wantTo('complete oauth flow with offline_access scope and obtain access_token and refresh_token'); + $authCode = $I->obtainAuthCode(['offline_access']); + $I->sendPOST('/api/oauth2/v1/token', [ + 'grant_type' => 'authorization_code', + 'code' => $authCode, + 'client_id' => 'ely', + 'client_secret' => 'ZuM1vGchJz-9_UZ5HC3H3Z9Hg5PzdbkM', + 'redirect_uri' => 'http://ely.by', + ]); + $I->canSeeResponseCodeIs(200); + $I->canSeeResponseContainsJson([ + 'token_type' => 'Bearer', + ]); + $I->canSeeResponseJsonMatchesJsonPath('$.access_token'); + $I->canSeeResponseJsonMatchesJsonPath('$.expires_in'); + $I->canSeeResponseJsonMatchesJsonPath('$.refresh_token'); + } + + public function callEndpointWithByEmptyRequest(OauthSteps $I) { + $I->wantTo('check behavior on on request without any params'); + $I->sendPOST('/api/oauth2/v1/token'); $I->canSeeResponseCodeIs(400); $I->canSeeResponseContainsJson([ - 'error' => 'invalid_request', - 'message' => 'The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Check the "grant_type" parameter.', + 'error' => 'unsupported_grant_type', + 'message' => 'The authorization grant type is not supported by the authorization server.', ]); + } + public function issueTokenByPassingInvalidAuthCode(OauthSteps $I) { $I->wantTo('check behavior on passing invalid auth code'); - $this->route->issueToken($this->buildParams( - 'wrong-auth-code', - 'ely', - 'ZuM1vGchJz-9_UZ5HC3H3Z9Hg5PzdbkM', - 'http://ely.by' - )); + $I->sendPOST('/api/oauth2/v1/token', [ + 'grant_type' => 'authorization_code', + 'code' => 'wrong-auth-code', + 'client_id' => 'ely', + 'client_secret' => 'ZuM1vGchJz-9_UZ5HC3H3Z9Hg5PzdbkM', + 'redirect_uri' => 'http://ely.by', + ]); $I->canSeeResponseCodeIs(400); $I->canSeeResponseContainsJson([ 'error' => 'invalid_request', 'message' => 'The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Check the "code" parameter.', ]); + } - $authCode = $I->getAuthCode(); + public function issueTokenByPassingInvalidRedirectUri(OauthSteps $I) { $I->wantTo('check behavior on passing invalid redirect_uri'); - $this->route->issueToken($this->buildParams( - $authCode, - 'ely', - 'ZuM1vGchJz-9_UZ5HC3H3Z9Hg5PzdbkM', - 'http://some-other.domain' - )); - $I->canSeeResponseCodeIs(401); + $authCode = $I->obtainAuthCode(); + $I->sendPOST('/api/oauth2/v1/token', [ + 'grant_type' => 'authorization_code', + 'code' => $authCode, + 'client_id' => 'ely', + 'client_secret' => 'ZuM1vGchJz-9_UZ5HC3H3Z9Hg5PzdbkM', + 'redirect_uri' => 'http://some-other.domain', + ]); + $I->canSeeResponseCodeIs(400); $I->canSeeResponseContainsJson([ 'error' => 'invalid_client', 'message' => 'Client authentication failed.', ]); } - public function testIssueToken(OauthSteps $I) { - $authCode = $I->getAuthCode(); - $this->route->issueToken($this->buildParams( - $authCode, - 'ely', - 'ZuM1vGchJz-9_UZ5HC3H3Z9Hg5PzdbkM', - 'http://ely.by' - )); - $I->canSeeResponseCodeIs(200); - $I->canSeeResponseIsJson(); - $I->canSeeResponseContainsJson([ - 'token_type' => 'Bearer', - ]); - $I->canSeeResponseJsonMatchesJsonPath('$.access_token'); - $I->cantSeeResponseJsonMatchesJsonPath('$.refresh_token'); - $I->canSeeResponseJsonMatchesJsonPath('$.expires_in'); - } - - public function testIssueTokenWithRefreshToken(OauthSteps $I) { - $authCode = $I->getAuthCode(['offline_access']); - $this->route->issueToken($this->buildParams( - $authCode, - 'ely', - 'ZuM1vGchJz-9_UZ5HC3H3Z9Hg5PzdbkM', - 'http://ely.by' - )); - $I->canSeeResponseCodeIs(200); - $I->canSeeResponseIsJson(); - $I->canSeeResponseContainsJson([ - 'token_type' => 'Bearer', - ]); - $I->canSeeResponseJsonMatchesJsonPath('$.access_token'); - $I->canSeeResponseJsonMatchesJsonPath('$.refresh_token'); - $I->canSeeResponseJsonMatchesJsonPath('$.expires_in'); - } - - private function buildParams($code = null, $clientId = null, $clientSecret = null, $redirectUri = null) { - $params = ['grant_type' => 'authorization_code']; - if ($code !== null) { - $params['code'] = $code; - } - - if ($clientId !== null) { - $params['client_id'] = $clientId; - } - - if ($clientSecret !== null) { - $params['client_secret'] = $clientSecret; - } - - if ($redirectUri !== null) { - $params['redirect_uri'] = $redirectUri; - } - - return $params; - } - } diff --git a/api/tests/functional/oauth/AuthCodeCest.php b/api/tests/functional/oauth/AuthCodeCest.php index fba1dd7..7041079 100644 --- a/api/tests/functional/oauth/AuthCodeCest.php +++ b/api/tests/functional/oauth/AuthCodeCest.php @@ -72,7 +72,7 @@ class AuthCodeCest { $I->canSeeResponseContainsJson([ 'success' => false, 'error' => 'accept_required', - 'parameter' => '', + 'parameter' => null, 'statusCode' => 401, ]); } @@ -90,7 +90,7 @@ class AuthCodeCest { $I->canSeeResponseContainsJson([ 'success' => false, 'error' => 'accept_required', - 'parameter' => '', + 'parameter' => null, 'statusCode' => 401, ]); } @@ -114,7 +114,7 @@ class AuthCodeCest { $I->canSeeResponseContainsJson([ 'success' => false, 'error' => 'accept_required', - 'parameter' => '', + 'parameter' => null, 'statusCode' => 401, ]); } diff --git a/common/components/Redis/Key.php b/common/components/Redis/Key.php index d48f420..c8b3be5 100644 --- a/common/components/Redis/Key.php +++ b/common/components/Redis/Key.php @@ -4,6 +4,9 @@ namespace common\components\Redis; use InvalidArgumentException; use Yii; +/** + * @deprecated + */ class Key { private $key; diff --git a/common/components/Redis/Set.php b/common/components/Redis/Set.php index b6a07ec..5023aea 100644 --- a/common/components/Redis/Set.php +++ b/common/components/Redis/Set.php @@ -5,6 +5,9 @@ use ArrayIterator; use IteratorAggregate; use Yii; +/** + * @deprecated + */ class Set extends Key implements IteratorAggregate { public function add($value): self { diff --git a/common/db/mysql/QueryBuilder.php b/common/db/mysql/QueryBuilder.php index 66c5a24..22cbbc2 100644 --- a/common/db/mysql/QueryBuilder.php +++ b/common/db/mysql/QueryBuilder.php @@ -1,8 +1,10 @@ db); + $result = new QueryBuilder($this->db); + $result->setExpressionBuilders([ + 'yii\db\JsonExpression' => JsonExpressionBuilder::class, + ]); + + return $result; } } diff --git a/common/models/Account.php b/common/models/Account.php index 49b240a..fc227b8 100644 --- a/common/models/Account.php +++ b/common/models/Account.php @@ -93,7 +93,7 @@ class Account extends ActiveRecord { } public function getOauthSessions(): ActiveQuery { - return $this->hasMany(OauthSession::class, ['owner_id' => 'id'])->andWhere(['owner_type' => 'user']); + return $this->hasMany(OauthSession::class, ['account_id' => 'id']); } public function getOauthClients(): OauthClientQuery { diff --git a/common/models/OauthSession.php b/common/models/OauthSession.php index cb98e33..2806cb7 100644 --- a/common/models/OauthSession.php +++ b/common/models/OauthSession.php @@ -1,38 +1,32 @@ TimestampBehavior::class, @@ -49,39 +43,28 @@ class OauthSession extends ActiveRecord { return $this->hasOne(Account::class, ['id' => 'owner_id']); } - public function getScopes(): Set { - return new Set(static::getDb()->getSchema()->getRawTableName(static::tableName()), $this->id, 'scopes'); - } + public function getScopes(): array { + if (empty($this->scopes) && $this->legacy_id !== null) { + return Yii::$app->redis->smembers($this->getLegacyRedisScopesKey()); + } - public function getAccessTokens() { - throw new NotSupportedException('This method is possible, but not implemented'); + return (array)$this->scopes; } public function beforeDelete(): bool { - if (!$result = parent::beforeDelete()) { - return $result; + if (!parent::beforeDelete()) { + return false; } - $this->clearScopes(); - $this->removeRefreshToken(); + if ($this->legacy_id !== null) { + Yii::$app->redis->del($this->getLegacyRedisScopesKey()); + } return true; } - public function removeRefreshToken(): void { - /** @var \api\components\OAuth2\Repositories\RefreshTokenStorage $refreshTokensStorage */ - // TODO: rework - $refreshTokensStorage = Yii::$app->oauth->getRefreshTokenStorage(); - $refreshTokensSet = $refreshTokensStorage->sessionHash($this->id); - foreach ($refreshTokensSet->members() as $refreshTokenId) { - $refreshTokensStorage->delete($refreshTokensStorage->get($refreshTokenId)); - } - - $refreshTokensSet->delete(); - } - - public function clearScopes(): void { - $this->getScopes()->delete(); + private function getLegacyRedisScopesKey(): string { + return "oauth:sessions:{$this->legacy_id}:scopes"; } } diff --git a/common/models/OauthSessionQuery.php b/common/models/OauthSessionQuery.php deleted file mode 100644 index ffd2949..0000000 --- a/common/models/OauthSessionQuery.php +++ /dev/null @@ -1,41 +0,0 @@ -primaryModel instanceof Account && $this->link === ['owner_id' => 'id']) { - $this->primaryModel->id = (string)$this->primaryModel->id; - $idHasBeenCastedToString = true; - } - - $query = parent::prepare($builder); - - if ($idHasBeenCastedToString) { - $this->primaryModel->id = (int)$this->primaryModel->id; - } - - return $query; - } - -} diff --git a/common/tests/fixtures/data/oauth-sessions.php b/common/tests/fixtures/data/oauth-sessions.php index 678833b..1075642 100644 --- a/common/tests/fixtures/data/oauth-sessions.php +++ b/common/tests/fixtures/data/oauth-sessions.php @@ -1,35 +1,27 @@ [ - 'id' => 1, - 'owner_type' => 'user', - 'owner_id' => 1, + 'account_id' => 1, 'client_id' => 'test1', - 'client_redirect_uri' => 'http://test1.net/oauth', + 'scopes' => null, 'created_at' => 1479944472, ], 'banned-account-session' => [ - 'id' => 2, - 'owner_type' => 'user', - 'owner_id' => 10, + 'account_id' => 10, 'client_id' => 'test1', - 'client_redirect_uri' => 'http://test1.net/oauth', + 'scopes' => null, 'created_at' => 1481421663, ], 'deleted-client-session' => [ - 'id' => 3, - 'owner_type' => 'user', - 'owner_id' => 1, + 'account_id' => 1, 'client_id' => 'deleted-oauth-client-with-sessions', - 'client_redirect_uri' => 'http://not-exists-site.com/oauth/ely', + 'scopes' => null, 'created_at' => 1519510065, ], 'actual-deleted-client-session' => [ - 'id' => 4, - 'owner_type' => 'user', - 'owner_id' => 2, + 'account_id' => 2, 'client_id' => 'deleted-oauth-client-with-sessions', - 'client_redirect_uri' => 'http://not-exists-site.com/oauth/ely', + 'scopes' => null, 'created_at' => 1519511568, ], ]; diff --git a/composer.json b/composer.json index fa8b7f5..d2451b9 100644 --- a/composer.json +++ b/composer.json @@ -24,6 +24,7 @@ "nesbot/carbon": "^2.22", "paragonie/constant_time_encoding": "^2.0", "ramsey/uuid": "^3.5", + "sam-it/yii2-mariadb": "^1.1", "spomky-labs/otphp": "^9.0.2", "webmozart/assert": "^1.2.0", "yiisoft/yii2": "~2.0.20", diff --git a/composer.lock b/composer.lock index 0cfc271..d5c2a2b 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "35a16287a6dc45c16e0553022aa34f5b", + "content-hash": "7ee1d380684b79ffabf92f115d7de4a8", "packages": [ { "name": "bacon/bacon-qr-code", @@ -1767,6 +1767,44 @@ ], "time": "2018-07-19T23:38:55+00:00" }, + { + "name": "sam-it/yii2-mariadb", + "version": "v1.1.0", + "source": { + "type": "git", + "url": "https://github.com/SAM-IT/yii2-mariadb.git", + "reference": "9d0551dc05332e51c4546b070e0cb221217d75d6" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/SAM-IT/yii2-mariadb/zipball/9d0551dc05332e51c4546b070e0cb221217d75d6", + "reference": "9d0551dc05332e51c4546b070e0cb221217d75d6", + "shasum": "" + }, + "require-dev": { + "friendsofphp/php-cs-fixer": "^2.9", + "phpunit/phpunit": "^7.2", + "yiisoft/yii2-dev": "^2.0.15" + }, + "type": "yii2-extension", + "autoload": { + "psr-4": { + "SamIT\\Yii2\\MariaDb\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Sam Mousa", + "email": "sam@mousa.nl" + } + ], + "description": "MariaDB Driver for Yii2", + "time": "2019-04-23T13:27:38+00:00" + }, { "name": "sentry/sentry", "version": "1.10.0", diff --git a/console/migrations/m190914_181236_rework_oauth_sessions_table.php b/console/migrations/m190914_181236_rework_oauth_sessions_table.php new file mode 100644 index 0000000..20cf5c5 --- /dev/null +++ b/console/migrations/m190914_181236_rework_oauth_sessions_table.php @@ -0,0 +1,57 @@ +delete('oauth_sessions', ['NOT', ['owner_type' => 'user']]); + $this->dropColumn('oauth_sessions', 'owner_type'); + $this->dropColumn('oauth_sessions', 'client_redirect_uri'); + $this->execute(' + DELETE os1 + FROM oauth_sessions os1, + oauth_sessions os2 + WHERE os1.id > os2.id + AND os1.owner_id = os2.owner_id + AND os1.client_id = os2.client_id + '); + $this->dropIndex('owner_id', 'oauth_sessions'); + $this->renameColumn('oauth_sessions', 'owner_id', 'account_id'); + $this->alterColumn('oauth_sessions', 'account_id', $this->db->getTableSchema('accounts')->getColumn('id')->dbType . ' NOT NULL'); + $this->alterColumn('oauth_sessions', 'client_id', $this->db->getTableSchema('oauth_clients')->getColumn('id')->dbType . ' NOT NULL'); + // Change type to be able to remove primary key + $this->alterColumn('oauth_sessions', 'id', $this->integer(11)->unsigned()->after('client_id')); + $this->dropPrimaryKey('PRIMARY', 'oauth_sessions'); + // Change type again to make column nullable + $this->alterColumn('oauth_sessions', 'id', $this->integer(11)->unsigned()->after('client_id')); + $this->renameColumn('oauth_sessions', 'id', 'legacy_id'); + $this->addPrimaryKey('id', 'oauth_sessions', ['account_id', 'client_id']); + $this->dropForeignKey('FK_oauth_session_to_client', 'oauth_sessions'); + $this->dropIndex('FK_oauth_session_to_client', 'oauth_sessions'); + $this->addForeignKey('FK_oauth_session_to_account', 'oauth_sessions', 'account_id', 'accounts', 'id', 'CASCADE', 'CASCADE'); + $this->addForeignKey('FK_oauth_session_to_oauth_client', 'oauth_sessions', 'client_id', 'oauth_clients', 'id', 'CASCADE', 'CASCADE'); + $this->addColumn('oauth_sessions', 'scopes', $this->json()->toString('scopes') . ' AFTER `legacy_id`'); + } + + public function safeDown() { + $this->dropColumn('oauth_sessions', 'scopes'); + $this->dropForeignKey('FK_oauth_session_to_oauth_client', 'oauth_sessions'); + $this->dropForeignKey('FK_oauth_session_to_account', 'oauth_sessions'); + $this->dropIndex('FK_oauth_session_to_oauth_client', 'oauth_sessions'); + $this->dropPrimaryKey('PRIMARY', 'oauth_sessions'); + $this->delete('oauth_sessions', ['legacy_id' => null]); + $this->alterColumn('oauth_sessions', 'legacy_id', $this->integer(11)->unsigned()->notNull()->append('AUTO_INCREMENT PRIMARY KEY FIRST')); + $this->renameColumn('oauth_sessions', 'legacy_id', 'id'); + $this->alterColumn('oauth_sessions', 'client_id', $this->db->getTableSchema('oauth_clients')->getColumn('id')->dbType); + $this->alterColumn('oauth_sessions', 'account_id', $this->string()); + $this->renameColumn('oauth_sessions', 'account_id', 'owner_id'); + $this->createIndex('owner_id', 'oauth_sessions', 'owner_id'); + $this->addColumn('oauth_sessions', 'owner_type', $this->string()->notNull()->after('id')); + $this->update('oauth_sessions', ['owner_type' => 'user']); + $this->addColumn('oauth_sessions', 'client_redirect_uri', $this->string()->after('client_id')); + $this->addForeignKey('FK_oauth_session_to_client', 'oauth_sessions', 'client_id', 'oauth_clients', 'id', 'CASCADE', 'CASCADE'); + } + +}