Feedback #1
Feedback #1
Conversation
![github-classroom[bot]](https://avatars.githubusercontent.com/in/68187?s=60&v=4){kind=small}
src/Service/TokenValidator.php
Outdated
| } | ||
| public function getPublicKeys($url): array | ||
| { | ||
| $this->curlClient->get($url); |
There was a problem hiding this comment.
Please double check the use of the property here.
src/Service/TokenValidator.php
Outdated
| public function getPublicKeys($url): array | ||
| { | ||
| $this->curlClient->get($url); | ||
| $jwks = json_decode($this->curlClient->getBody(), true); |
There was a problem hiding this comment.
We're going to add one improvement here: the Magento framework ships with a json encoder/decoder class that does this same thing, but adds a layer of error protection on top. I'll let you search for it yourself, but if you can't find it after 15 minutes of searching let me know and I'll show you.
The general principle is: we should (almost) never use native PHP function calls in our code, because there's probably a utility from either the Magento Framework or Zend framework that will add important error protection functionality on top.
In the case of \json_decode() for example, if the argument is invalid JSON, the core PHP function will just return false. In order to know WHAT the error actually was, you need to catch that use-case (compare the result with false) and then call the \json_last_error_msg(). So therefore, only calling json_decode is not enough because there's at least one use-case that's not being covered: if things go wrong. By using the Magento Framework implementation, you get that error handling mechanism already implemented for free: your code is suddenly a lot more "defensive", which essentially means it's much harder to "break" your code / cause a bug.
src/Service/TokenValidator.php
Outdated
| use \Firebase\JWT\JWT; | ||
| use \Firebase\JWT\JWK; | ||
|
|
||
| class TokenValidator { |
There was a problem hiding this comment.
Good work refactoring to this class! We will now start to make the code a lot more "defensive", so I'll add some comments on how to do that.
src/Plugin/FrontControllerPlugin.php
Outdated
| { | ||
| public const TEAM_DOMAIN = 'https://hyva.ancord.io'; | ||
| public const CERTS_URL = self::TEAM_DOMAIN .'/cdn-cgi/access/certs'; | ||
| public const ALGORITHM = 'RS256'; |
There was a problem hiding this comment.
You can move these constants into the TokenValidator service.
src/Plugin/FrontControllerPlugin.php
Outdated
|
|
||
| public function aroundDispatch(FrontController $subject, callable $proceed) | ||
| { | ||
| $payload = []; //$this->getMockPayload(); |
There was a problem hiding this comment.
- Get $request from
FrontController - Get cookies from $request.
- Get $token from
CF_Authorizationcookie (you can rename$payloadto$token).
src/Plugin/FrontControllerPlugin.php
Outdated
| } | ||
|
|
||
| try { | ||
| $result = $this->serviceToken->getJWT($payload, self::CERTS_URL, self::ALGORITHM); |
There was a problem hiding this comment.
Better name for $result => $validatedToken
src/Plugin/FrontControllerPlugin.php
Outdated
| public const CERTS_URL = self::TEAM_DOMAIN .'/cdn-cgi/access/certs'; | ||
| public const ALGORITHM = 'RS256'; | ||
|
|
||
| protected $serviceToken; |
src/Service/TokenValidator.php
Outdated
| return $jwks; | ||
| } | ||
|
|
||
| public function getJWT($payload, $url, $algorithm) { |
There was a problem hiding this comment.
Some improvements:
public function validateToken(string $token, string $url, string $algorithm): object // fijate bien el result que sea igual al de JWT::decode
👋! GitHub Classroom created this pull request as a place for your teacher to leave feedback on your work. It will update automatically. Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.
In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.
Click the Files changed or Commits tab to see all of the changes pushed to
mastersince the assignment started. Your teacher can see this too.Notes for teachers
Use this PR to leave feedback. Here are some tips:
mastersince the assignment started. To leave comments on specific lines of code, put your cursor over a line of code and click the blue + (plus sign). To learn more about comments, read “Commenting on a pull request”.master. Click a commit to see specific changes.For more information about this pull request, read “Leaving assignment feedback in GitHub”.
Subscribed: @mparatcha