diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 8fa9b4f5..cac3bf82 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -1,10 +1,13 @@ -name: Tests & style checks +name: Tests & Style Checks on: - # Trigger on any PR being opened, or on a merge to master (to update the badge) + # Trigger on any PR being opened pull_request: + # Or weekly and on a merge to master (to update the badge) push: branches: - master + schedule: + - cron: 0 0 * * 0 jobs: lint: name: Lint @@ -34,3 +37,19 @@ jobs: composer-options: "${{ matrix.composer-options }}" - name: Run tests run: composer test + + coverage: + name: Code Coverage + runs-on: ubuntu-latest + steps: + - uses: "actions/checkout@v3" + - uses: "shivammathur/setup-php@v2" + with: + php-version: latest + coverage: xdebug + - uses: "ramsey/composer-install@v2" + - uses: paambaati/codeclimate-action@v5.0.0 + env: + CC_TEST_REPORTER_ID: ${{ secrets.CC_TEST_REPORTER_ID }} + with: + coverageCommand: composer test diff --git a/.gitignore b/.gitignore index 2cfc769b..029b0f92 100644 --- a/.gitignore +++ b/.gitignore @@ -15,4 +15,5 @@ tests/_support/_generated/* vendor # ignore the coverage folders +**/.phpunit.cache **/coverage diff --git a/README.md b/README.md index db1fb320..5dd75511 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # LTI 1.3 Tool Library -![Test status](https://github.com/packbackbooks/lti-1-3-php-library/actions/workflows/run_tests.yml/badge.svg?branch=master) +![Test status](https://github.com/packbackbooks/lti-1-3-php-library/actions/workflows/run_tests.yml/badge.svg?branch=master) [![Maintainability](https://api.codeclimate.com/v1/badges/16055e83ea04ad95a2f9/maintainability)](https://codeclimate.com/github/packbackbooks/lti-1-3-php-library/maintainability) [![Test Coverage](https://api.codeclimate.com/v1/badges/16055e83ea04ad95a2f9/test_coverage)](https://codeclimate.com/github/packbackbooks/lti-1-3-php-library/test_coverage) A library used for building IMS-certified LTI 1.3 tool providers in PHP. diff --git a/composer.json b/composer.json index 183bcbf7..1e8a691f 100644 --- a/composer.json +++ b/composer.json @@ -30,7 +30,7 @@ "nesbot/carbon": "^2.43", "laravel/pint": "^1.0", "phpstan/phpstan": "^1.10", - "phpunit/phpunit": "^9.5" + "phpunit/phpunit": "^9.0|^10.0" }, "autoload": { "psr-4": { @@ -43,11 +43,11 @@ } }, "scripts": { - "test": [ - "phpunit", + "test": "phpunit", + "lint": [ + "pint --test", "phpstan analyse" ], - "lint": "pint -v --test", "lint-fix": "pint -v" } } diff --git a/phpunit.xml.dist b/phpunit.xml.dist index daa8b21a..1fff419b 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,9 +1,6 @@ - + - - src/ - @@ -18,4 +15,9 @@ + + + src/ + + diff --git a/src/Concerns/Arrayable.php b/src/Concerns/Arrayable.php new file mode 100644 index 00000000..c83f345e --- /dev/null +++ b/src/Concerns/Arrayable.php @@ -0,0 +1,15 @@ +getArray()); + } +} diff --git a/src/Concerns/JsonStringable.php b/src/Concerns/JsonStringable.php new file mode 100644 index 00000000..29242827 --- /dev/null +++ b/src/Concerns/JsonStringable.php @@ -0,0 +1,13 @@ +toArray()); + } +} diff --git a/src/DeepLinkResources/DateTimeInterval.php b/src/DeepLinkResources/DateTimeInterval.php index 82b1aa6e..e3b2cef2 100644 --- a/src/DeepLinkResources/DateTimeInterval.php +++ b/src/DeepLinkResources/DateTimeInterval.php @@ -3,10 +3,12 @@ namespace Packback\Lti1p3\DeepLinkResources; use DateTime; +use Packback\Lti1p3\Concerns\Arrayable; use Packback\Lti1p3\LtiException; class DateTimeInterval { + use Arrayable; public const ERROR_NO_START_OR_END = 'Either a start or end time must be specified.'; public const ERROR_START_GT_END = 'The start time cannot be greater than end time.'; @@ -22,6 +24,20 @@ public static function new(): self return new DateTimeInterval(); } + public function getArray(): array + { + if (!isset($this->start) && !isset($this->end)) { + throw new LtiException(self::ERROR_NO_START_OR_END); + } + + $this->validateStartAndEnd(); + + return [ + 'startDateTime' => $this->start?->format(DateTime::ATOM), + 'endDateTime' => $this->end?->format(DateTime::ATOM), + ]; + } + public function setStart(?DateTime $start): self { $this->start = $start; @@ -46,26 +62,9 @@ public function getEnd(): ?DateTime return $this->end; } - public function toArray(): array - { - if (!isset($this->start) && !isset($this->end)) { - throw new LtiException(self::ERROR_NO_START_OR_END); - } - - $this->validateStartAndEnd(); - - $dateTimeInterval = []; - - if (isset($this->start)) { - $dateTimeInterval['startDateTime'] = $this->start->format(DateTime::ATOM); - } - if (isset($this->end)) { - $dateTimeInterval['endDateTime'] = $this->end->format(DateTime::ATOM); - } - - return $dateTimeInterval; - } - + /** + * @throws LtiException + */ private function validateStartAndEnd(): void { if (isset($this->start) && isset($this->end) && $this->start > $this->end) { diff --git a/src/DeepLinkResources/Icon.php b/src/DeepLinkResources/Icon.php index 84ae4e16..267c983f 100644 --- a/src/DeepLinkResources/Icon.php +++ b/src/DeepLinkResources/Icon.php @@ -2,9 +2,11 @@ namespace Packback\Lti1p3\DeepLinkResources; +use Packback\Lti1p3\Concerns\Arrayable; + class Icon { - use HasDimensions; + use Arrayable, HasDimensions; public function __construct( private string $url, @@ -18,6 +20,15 @@ public static function new(string $url, int $width, int $height): self return new Icon($url, $width, $height); } + public function getArray(): array + { + return [ + 'url' => $this->url, + 'width' => $this->width, + 'height' => $this->height, + ]; + } + public function setUrl(string $url): self { $this->url = $url; @@ -29,13 +40,4 @@ public function getUrl(): string { return $this->url; } - - public function toArray(): array - { - return [ - 'url' => $this->url, - 'width' => $this->width, - 'height' => $this->height, - ]; - } } diff --git a/src/DeepLinkResources/Iframe.php b/src/DeepLinkResources/Iframe.php index 8120f2fa..039ea8eb 100644 --- a/src/DeepLinkResources/Iframe.php +++ b/src/DeepLinkResources/Iframe.php @@ -2,9 +2,11 @@ namespace Packback\Lti1p3\DeepLinkResources; +use Packback\Lti1p3\Concerns\Arrayable; + class Iframe { - use HasDimensions; + use Arrayable, HasDimensions; public function __construct( private ?string $src = null, @@ -18,6 +20,15 @@ public static function new(): self return new Iframe(); } + public function getArray(): array + { + return [ + 'width' => $this->width, + 'height' => $this->height, + 'src' => $this->src, + ]; + } + public function setSrc(?string $src): self { $this->src = $src; @@ -29,21 +40,4 @@ public function getSrc(): ?string { return $this->src; } - - public function toArray(): array - { - $iframe = []; - - if (isset($this->width)) { - $iframe['width'] = $this->width; - } - if (isset($this->height)) { - $iframe['height'] = $this->height; - } - if (isset($this->src)) { - $iframe['src'] = $this->src; - } - - return $iframe; - } } diff --git a/src/DeepLinkResources/Resource.php b/src/DeepLinkResources/Resource.php index 9bfc7802..5cbb4b2b 100644 --- a/src/DeepLinkResources/Resource.php +++ b/src/DeepLinkResources/Resource.php @@ -2,11 +2,13 @@ namespace Packback\Lti1p3\DeepLinkResources; +use Packback\Lti1p3\Concerns\Arrayable; use Packback\Lti1p3\LtiConstants; use Packback\Lti1p3\LtiLineitem; class Resource { + use Arrayable; private string $type = LtiConstants::DL_RESOURCE_LINK_TYPE; private ?string $title = null; private ?string $text = null; @@ -26,6 +28,42 @@ public static function new(): self return new Resource(); } + public function getArray(): array + { + $resource = [ + 'type' => $this->type, + 'title' => $this->title, + 'text' => $this->text, + 'url' => $this->url, + 'icon' => $this->icon?->toArray(), + 'thumbnail' => $this->thumbnail?->toArray(), + 'iframe' => $this->iframe?->toArray(), + 'window' => $this->window?->toArray(), + 'available' => $this->availability_interval?->toArray(), + 'submission' => $this->submission_interval?->toArray(), + ]; + + if (!empty($this->custom_params)) { + $resource['custom'] = $this->custom_params; + } + + if (isset($this->line_item)) { + $resource['lineItem'] = [ + 'scoreMaximum' => $this->line_item->getScoreMaximum(), + 'label' => $this->line_item->getLabel(), + ]; + } + + // Kept for backwards compatibility + if (!isset($this->iframe) && !isset($this->window)) { + $resource['presentation'] = [ + 'documentTarget' => $this->target, + ]; + } + + return $resource; + } + public function getType(): string { return $this->type; @@ -169,58 +207,4 @@ public function setSubmissionInterval(?DateTimeInterval $submissionInterval): se return $this; } - - public function toArray(): array - { - $resource = [ - 'type' => $this->type, - ]; - - if (isset($this->title)) { - $resource['title'] = $this->title; - } - if (isset($this->text)) { - $resource['text'] = $this->text; - } - if (isset($this->url)) { - $resource['url'] = $this->url; - } - if (!empty($this->custom_params)) { - $resource['custom'] = $this->custom_params; - } - if (isset($this->icon)) { - $resource['icon'] = $this->icon->toArray(); - } - if (isset($this->thumbnail)) { - $resource['thumbnail'] = $this->thumbnail->toArray(); - } - if ($this->line_item !== null) { - $resource['lineItem'] = [ - 'scoreMaximum' => $this->line_item->getScoreMaximum(), - 'label' => $this->line_item->getLabel(), - ]; - } - - // Kept for backwards compatibility - if (!isset($this->iframe) && !isset($this->window)) { - $resource['presentation'] = [ - 'documentTarget' => $this->target, - ]; - } - - if (isset($this->iframe)) { - $resource['iframe'] = $this->iframe->toArray(); - } - if (isset($this->window)) { - $resource['window'] = $this->window->toArray(); - } - if (isset($this->availability_interval)) { - $resource['available'] = $this->availability_interval->toArray(); - } - if (isset($this->submission_interval)) { - $resource['submission'] = $this->submission_interval->toArray(); - } - - return $resource; - } } diff --git a/src/DeepLinkResources/Window.php b/src/DeepLinkResources/Window.php index 3ba8831a..7ee30ba0 100644 --- a/src/DeepLinkResources/Window.php +++ b/src/DeepLinkResources/Window.php @@ -2,9 +2,11 @@ namespace Packback\Lti1p3\DeepLinkResources; +use Packback\Lti1p3\Concerns\Arrayable; + class Window { - use HasDimensions; + use Arrayable, HasDimensions; public function __construct( private ?string $target_name = null, @@ -19,6 +21,16 @@ public static function new(): self return new Window(); } + public function getArray(): array + { + return [ + 'targetName' => $this->target_name, + 'width' => $this->width, + 'height' => $this->height, + 'windowFeatures' => $this->window_features, + ]; + } + public function setTargetName(?string $targetName): self { $this->target_name = $targetName; @@ -42,24 +54,4 @@ public function getWindowFeatures(): ?string { return $this->window_features; } - - public function toArray(): array - { - $window = []; - - if (isset($this->target_name)) { - $window['targetName'] = $this->target_name; - } - if (isset($this->width)) { - $window['width'] = $this->width; - } - if (isset($this->height)) { - $window['height'] = $this->height; - } - if (isset($this->window_features)) { - $window['windowFeatures'] = $this->window_features; - } - - return $window; - } } diff --git a/src/Helpers/Helpers.php b/src/Helpers/Helpers.php index 39fb9611..ad64d43f 100644 --- a/src/Helpers/Helpers.php +++ b/src/Helpers/Helpers.php @@ -4,9 +4,9 @@ class Helpers { - public static function checkIfNullValue($value): bool + public static function filterOutNulls(array $array): array { - return !is_null($value); + return array_filter($array, fn ($value) => !is_null($value)); } public static function buildUrlWithQueryParams(string $url, array $params = []): string diff --git a/src/Interfaces/ILtiServiceConnector.php b/src/Interfaces/ILtiServiceConnector.php index cb3d073b..9983d658 100644 --- a/src/Interfaces/ILtiServiceConnector.php +++ b/src/Interfaces/ILtiServiceConnector.php @@ -2,16 +2,18 @@ namespace Packback\Lti1p3\Interfaces; -use GuzzleHttp\Psr7\Response; +use Psr\Http\Message\ResponseInterface; /** @internal */ interface ILtiServiceConnector { - public function getAccessToken(ILtiRegistration $registration, array $scopes); + public function getAccessToken(ILtiRegistration $registration, array $scopes): string; - public function makeRequest(IServiceRequest $request); + public function makeRequest(IServiceRequest $request): ResponseInterface; - public function getResponseBody(Response $request): ?array; + public function getResponseBody(ResponseInterface $response): ?array; + + public function getResponseHeaders(ResponseInterface $response): ?array; public function makeServiceRequest( ILtiRegistration $registration, diff --git a/src/Interfaces/IServiceRequest.php b/src/Interfaces/IServiceRequest.php index 6404ce03..d619b159 100644 --- a/src/Interfaces/IServiceRequest.php +++ b/src/Interfaces/IServiceRequest.php @@ -17,6 +17,8 @@ public function setAccessToken(string $accessToken): IServiceRequest; public function setBody(string $body): IServiceRequest; + public function setPayload(array $payload): IServiceRequest; + public function setAccept(string $accept): IServiceRequest; public function setContentType(string $contentType): IServiceRequest; diff --git a/src/Lti1p1Key.php b/src/Lti1p1Key.php index 4abd70bf..091f9fc5 100644 --- a/src/Lti1p1Key.php +++ b/src/Lti1p1Key.php @@ -23,7 +23,7 @@ public function getKey(): ?string return $this->key; } - public function setKey(string $key) + public function setKey(string $key): self { $this->key = $key; @@ -35,7 +35,7 @@ public function getSecret(): ?string return $this->secret; } - public function setSecret(string $secret) + public function setSecret(string $secret): self { $this->secret = $secret; diff --git a/src/LtiAbstractService.php b/src/LtiAbstractService.php index 56f28d06..d0614ba4 100644 --- a/src/LtiAbstractService.php +++ b/src/LtiAbstractService.php @@ -32,7 +32,7 @@ abstract public function getScope(): array; protected function validateScopes(array $scopes): void { if (empty(array_intersect($scopes, $this->getScope()))) { - throw new LtiException('Missing required scope', 1); + throw new LtiException('Missing required scope'); } } diff --git a/src/LtiDeepLink.php b/src/LtiDeepLink.php index 7fbdbc15..a1fae707 100644 --- a/src/LtiDeepLink.php +++ b/src/LtiDeepLink.php @@ -10,8 +10,8 @@ class LtiDeepLink public function __construct( private ILtiRegistration $registration, private string $deployment_id, - private array $deep_link_settings) - { + private array $deep_link_settings + ) { } public function getResponseJwt(array $resources): string diff --git a/src/LtiDeployment.php b/src/LtiDeployment.php index 09caacbf..7ac23989 100644 --- a/src/LtiDeployment.php +++ b/src/LtiDeployment.php @@ -11,7 +11,7 @@ public function __construct( ) { } - public static function new($deployment_id) + public static function new($deployment_id): self { return new LtiDeployment($deployment_id); } @@ -21,7 +21,7 @@ public function getDeploymentId() return $this->deployment_id; } - public function setDeploymentId($deployment_id): LtiDeployment + public function setDeploymentId($deployment_id): self { $this->deployment_id = $deployment_id; diff --git a/src/LtiGrade.php b/src/LtiGrade.php index 67654420..bd673b4e 100644 --- a/src/LtiGrade.php +++ b/src/LtiGrade.php @@ -2,8 +2,11 @@ namespace Packback\Lti1p3; +use Packback\Lti1p3\Concerns\JsonStringable; + class LtiGrade { + use JsonStringable; private $score_given; private $score_maximum; private $comment; @@ -27,10 +30,9 @@ public function __construct(?array $grade = null) $this->canvas_extension = $grade['https://canvas.instructure.com/lti/submission'] ?? null; } - public function __toString(): string + public function getArray(): array { - // Additionally, includes the call back to filter out only NULL values - $request = array_filter([ + return [ 'scoreGiven' => $this->score_given, 'scoreMaximum' => $this->score_maximum, 'comment' => $this->comment, @@ -40,9 +42,7 @@ public function __toString(): string 'userId' => $this->user_id, 'submissionReview' => $this->submission_review, 'https://canvas.instructure.com/lti/submission' => $this->canvas_extension, - ], '\Packback\Lti1p3\Helpers\Helpers::checkIfNullValue'); - - return json_encode($request); + ]; } /** diff --git a/src/LtiGradeSubmissionReview.php b/src/LtiGradeSubmissionReview.php index b4f0b7cd..4732d784 100644 --- a/src/LtiGradeSubmissionReview.php +++ b/src/LtiGradeSubmissionReview.php @@ -2,8 +2,11 @@ namespace Packback\Lti1p3; +use Packback\Lti1p3\Concerns\JsonStringable; + class LtiGradeSubmissionReview { + use JsonStringable; private $reviewable_status; private $label; private $url; @@ -17,15 +20,14 @@ public function __construct(?array $gradeSubmission = null) $this->custom = $gradeSubmission['custom'] ?? null; } - public function __toString(): string + public function getArray(): array { - // Additionally, includes the call back to filter out only NULL values - return json_encode(array_filter([ + return [ 'reviewableStatus' => $this->reviewable_status, 'label' => $this->label, 'url' => $this->url, 'custom' => $this->custom, - ], '\Packback\Lti1p3\Helpers\Helpers::checkIfNullValue')); + ]; } /** diff --git a/src/LtiLineitem.php b/src/LtiLineitem.php index 0f2485d9..c8e662ed 100644 --- a/src/LtiLineitem.php +++ b/src/LtiLineitem.php @@ -2,8 +2,11 @@ namespace Packback\Lti1p3; +use Packback\Lti1p3\Concerns\JsonStringable; + class LtiLineitem { + use JsonStringable; private $id; private $score_maximum; private $label; @@ -27,10 +30,17 @@ public function __construct(?array $lineitem = null) $this->grades_released = $lineitem['gradesReleased'] ?? null; } - public function __toString(): string + /** + * Static function to allow for method chaining without having to assign to a variable first. + */ + public static function new(?array $lineItem = null): self + { + return new LtiLineitem($lineItem); + } + + public function getArray(): array { - // Additionally, includes the call back to filter out only NULL values - return json_encode(array_filter([ + return [ 'id' => $this->id, 'scoreMaximum' => $this->score_maximum, 'label' => $this->label, @@ -40,15 +50,7 @@ public function __toString(): string 'startDateTime' => $this->start_date_time, 'endDateTime' => $this->end_date_time, 'gradesReleased' => $this->grades_released, - ], '\Packback\Lti1p3\Helpers\Helpers::checkIfNullValue')); - } - - /** - * Static function to allow for method chaining without having to assign to a variable first. - */ - public static function new(?array $lineItem = null): self - { - return new LtiLineitem($lineItem); + ]; } public function getId() diff --git a/src/LtiMessageLaunch.php b/src/LtiMessageLaunch.php index e6f8ae32..ac179b5a 100644 --- a/src/LtiMessageLaunch.php +++ b/src/LtiMessageLaunch.php @@ -84,7 +84,7 @@ public static function new( ICache $cache, ICookie $cookie, ILtiServiceConnector $serviceConnector - ) { + ): self { return new LtiMessageLaunch($db, $cache, $cookie, $serviceConnector); } @@ -99,7 +99,7 @@ public static function fromCache( ICache $cache, ICookie $cookie, ILtiServiceConnector $serviceConnector - ) { + ): self { $new = new LtiMessageLaunch($db, $cache, $cookie, $serviceConnector); $new->launch_id = $launch_id; $new->jwt = ['body' => $new->cache->getLaunchData($launch_id)]; @@ -169,7 +169,7 @@ public function cacheLaunchData(): self */ public function hasNrps(): bool { - return !empty($this->jwt['body'][LtiConstants::NRPS_CLAIM_SERVICE]['context_memberships_url']); + return isset($this->jwt['body'][LtiConstants::NRPS_CLAIM_SERVICE]['context_memberships_url']); } /** @@ -189,7 +189,7 @@ public function getNrps(): LtiNamesRolesProvisioningService */ public function hasGs(): bool { - return !empty($this->jwt['body'][LtiConstants::GS_CLAIM_SERVICE]['context_groups_url']); + return isset($this->jwt['body'][LtiConstants::GS_CLAIM_SERVICE]['context_groups_url']); } /** @@ -209,7 +209,7 @@ public function getGs(): LtiCourseGroupsService */ public function hasAgs(): bool { - return !empty($this->jwt['body'][LtiConstants::AGS_CLAIM_ENDPOINT]); + return isset($this->jwt['body'][LtiConstants::AGS_CLAIM_ENDPOINT]); } /** @@ -375,14 +375,12 @@ protected function validateState(): self protected function validateJwtFormat(): self { - $jwt = $this->request['id_token'] ?? null; - - if (empty($jwt)) { + if (!isset($this->request['id_token'])) { throw new LtiException(static::ERR_MISSING_ID_TOKEN); } // Get parts of JWT. - $jwt_parts = explode('.', $jwt); + $jwt_parts = explode('.', $this->request['id_token']); if (count($jwt_parts) !== 3) { // Invalid number of parts in JWT. @@ -416,7 +414,7 @@ protected function validateRegistration(): self $issuerUrl = $this->jwt['body']['iss']; $this->registration = $this->db->findRegistrationByIssuer($issuerUrl, $clientId); - if (empty($this->registration)) { + if (!isset($this->registration)) { throw new LtiException($this->getMissingRegistrationErrorMsg($issuerUrl, $clientId)); } @@ -469,7 +467,7 @@ protected function validateDeployment(): self protected function validateMessage(): self { - if (empty($this->jwt['body'][LtiConstants::MESSAGE_TYPE])) { + if (!isset($this->jwt['body'][LtiConstants::MESSAGE_TYPE])) { // Unable to identify message type. throw new LtiException(static::ERR_INVALID_MESSAGE_TYPE); } diff --git a/src/LtiOidcLogin.php b/src/LtiOidcLogin.php index b2b10461..7b9dc7f8 100644 --- a/src/LtiOidcLogin.php +++ b/src/LtiOidcLogin.php @@ -18,90 +18,82 @@ class LtiOidcLogin public function __construct( public IDatabase $db, public ICache $cache, - public ICookie $cookie) - { + public ICookie $cookie + ) { } /** * Static function to allow for method chaining without having to assign to a variable first. */ - public static function new(IDatabase $db, ICache $cache, ICookie $cookie) + public static function new(IDatabase $db, ICache $cache, ICookie $cookie): self { return new LtiOidcLogin($db, $cache, $cookie); } /** * Calculate the redirect location to return to based on an OIDC third party initiated login request. - * - * @param string $launchUrl URL to redirect back to after the OIDC login. This URL must match exactly a URL white listed in the platform. - * @param array $request An array of request parameters. - * @return string returns the fully formed OIDC login URL */ public function getRedirectUrl(string $launchUrl, array $request): string { - // Validate Request Data. + // Validate request data. $registration = $this->validateOidcLogin($request); - /* - * Build OIDC Auth Response. - */ + // Build OIDC Auth response. + $authParams = $this->getAuthParams($launchUrl, $registration->getClientId(), $request); + + return Helpers::buildUrlWithQueryParams($registration->getAuthLoginUrl(), $authParams); + } + + public function validateOidcLogin(array $request): ILtiRegistration + { + if (!isset($request['iss'])) { + throw new OidcException(static::ERROR_MSG_ISSUER); + } + + if (!isset($request['login_hint'])) { + throw new OidcException(static::ERROR_MSG_LOGIN_HINT); + } + + // Fetch registration + $clientId = $request['client_id'] ?? null; + $registration = $this->db->findRegistrationByIssuer($request['iss'], $clientId); + + if (!isset($registration)) { + $errorMsg = LtiMessageLaunch::getMissingRegistrationErrorMsg($request['iss'], $clientId); + + throw new OidcException($errorMsg); + } + + return $registration; + } - // Generate State. + public function getAuthParams(string $launchUrl, string $clientId, array $request): array + { // Set cookie (short lived) $state = static::secureRandomString('state-'); $this->cookie->setCookie(static::COOKIE_PREFIX.$state, $state, 60); - // Generate Nonce. $nonce = static::secureRandomString('nonce-'); $this->cache->cacheNonce($nonce, $state); - // Build Response. $authParams = [ 'scope' => 'openid', // OIDC Scope. 'response_type' => 'id_token', // OIDC response is always an id token. 'response_mode' => 'form_post', // OIDC response is always a form post. 'prompt' => 'none', // Don't prompt user on redirect. - 'client_id' => $registration->getClientId(), // Registered client id. + 'client_id' => $clientId, // Registered client id. 'redirect_uri' => $launchUrl, // URL to return to after login. 'state' => $state, // State to identify browser session. 'nonce' => $nonce, // Prevent replay attacks. 'login_hint' => $request['login_hint'], // Login hint to identify platform session. ]; - // Pass back LTI message hint if we have it. if (isset($request['lti_message_hint'])) { // LTI message hint to identify LTI context within the platform. $authParams['lti_message_hint'] = $request['lti_message_hint']; } - return Helpers::buildUrlWithQueryParams($registration->getAuthLoginUrl(), $authParams); - } - - public function validateOidcLogin(array $request): ILtiRegistration - { - // Validate Issuer. - if (empty($request['iss'])) { - throw new OidcException(static::ERROR_MSG_ISSUER, 1); - } - - // Validate Login Hint. - if (empty($request['login_hint'])) { - throw new OidcException(static::ERROR_MSG_LOGIN_HINT, 1); - } - - // Fetch Registration Details. - $clientId = $request['client_id'] ?? null; - $registration = $this->db->findRegistrationByIssuer($request['iss'], $clientId); - - // Check we got something. - if (empty($registration)) { - $errorMsg = LtiMessageLaunch::getMissingRegistrationErrorMsg($request['iss'], $clientId); - - throw new OidcException($errorMsg, 1); - } - - // Return Registration. - return $registration; + return $authParams; } public static function secureRandomString(string $prefix = ''): string diff --git a/src/LtiRegistration.php b/src/LtiRegistration.php index ec1293b9..0bc4089b 100644 --- a/src/LtiRegistration.php +++ b/src/LtiRegistration.php @@ -94,7 +94,7 @@ public function setAuthLoginUrl(string $authLoginUrl): self public function getAuthServer() { - return empty($this->authServer) ? $this->authTokenUrl : $this->authServer; + return $this->authServer ?? $this->authTokenUrl; } public function setAuthServer(string $authServer): self diff --git a/src/LtiServiceConnector.php b/src/LtiServiceConnector.php index e9d76fca..0a73553b 100644 --- a/src/LtiServiceConnector.php +++ b/src/LtiServiceConnector.php @@ -36,7 +36,8 @@ public function getAccessToken(ILtiRegistration $registration, array $scopes): s $accessTokenKey = $this->getAccessTokenCacheKey($registration, $scopes); // Get access token from cache if it exists $accessToken = $this->cache->getAccessToken($accessTokenKey); - if ($accessToken) { + + if (isset($accessToken)) { return $accessToken; } @@ -164,7 +165,12 @@ public function getAll( while ($nextUrl) { $response = $this->makeServiceRequest($registration, $scopes, $request); - $page_results = $key === null ? ($response['body'] ?? []) : ($response['body'][$key] ?? []); + if (isset($key)) { + $page_results = $response['body'][$key] ?? []; + } else { + $page_results = $response['body'] ?? []; + } + $results = array_merge($results, $page_results); $nextUrl = $this->getNextUrl($response['headers']); @@ -195,7 +201,7 @@ public static function getLogMessage( $requestBody = $request->getPayload()['body'] ?? null; - if (!empty($requestBody)) { + if (isset($requestBody)) { $contextArray['request_body'] = $requestBody; } diff --git a/src/ServiceRequest.php b/src/ServiceRequest.php index 8c8edffd..044d3eb5 100644 --- a/src/ServiceRequest.php +++ b/src/ServiceRequest.php @@ -46,8 +46,8 @@ class ServiceRequest implements IServiceRequest public function __construct( private string $method, private string $url, - private $type = self::TYPE_UNSUPPORTED) - { + private string $type = self::TYPE_UNSUPPORTED + ) { } public function getMethod(): string diff --git a/tests/LtiOidcLoginTest.php b/tests/LtiOidcLoginTest.php index 42b8ca26..805878e9 100644 --- a/tests/LtiOidcLoginTest.php +++ b/tests/LtiOidcLoginTest.php @@ -112,7 +112,37 @@ public function testValidatesFailsIfRegistrationNotFound() $this->oidcLogin->validateOidcLogin($request); } - /* - * @todo Finish testing - */ + public function testGetAuthParams() + { + $this->cookie->shouldReceive('setCookie') + ->once(); + $this->cache->shouldReceive('cacheNonce') + ->once(); + + $launchUrl = 'https://example.com/launch'; + $clientId = 'ClientId'; + $expected = [ + 'scope' => 'openid', + 'response_type' => 'id_token', + 'response_mode' => 'form_post', + 'prompt' => 'none', + 'client_id' => $clientId, + 'redirect_uri' => $launchUrl, + 'login_hint' => 'LoginHint', + 'lti_message_hint' => 'LtiMessageHint', + ]; + $request = [ + 'login_hint' => 'LoginHint', + 'lti_message_hint' => 'LtiMessageHint', + ]; + + $result = $this->oidcLogin->getAuthParams($launchUrl, $clientId, $request); + + // These are cryptographically random, so just assert they exist + $this->assertArrayHasKey('state', $result); + $this->assertArrayHasKey('nonce', $result); + // No remove them and check equality + unset($result['state'], $result['nonce']); + $this->assertEquals($expected, $result); + } }