feat: added support for API tokens authentication with optional automatic token rotation and storage layer (APPS-16010) - #85
Open
jszymanski-rtbh wants to merge 2 commits into
Conversation
jszymanski-rtbh
commented
Jul 3, 2026
Comment on lines
+128
to
+141
| { | ||
| if ($this->_auth instanceof ApiTokenAuth) { | ||
| return $this->_createStaticTokenClient($this->_auth); | ||
| } | ||
|
|
||
| if ($this->_auth instanceof DynamicApiTokenAuth) { | ||
| return $this->_createDynamicTokenClient($this->_auth); | ||
| } | ||
|
|
||
| if ($this->_auth instanceof CookieAuth) { | ||
| return $this->_createCookieAuthenticatedClient($this->_auth); | ||
| } | ||
| throw new ReportsApiException('Unsupported authentication method: ' . get_class($this->_auth)); | ||
| } |
Author
There was a problem hiding this comment.
match case jest od php 8
…atic token rotation and storage layer (APPS-16010)
…p-sdk into APPS-16010-api-tokens-authentication-support
jszymanski-rtbh
force-pushed
the
APPS-16010-api-tokens-authentication-support
branch
from
July 3, 2026 17:32
3db41b6 to
7037bf9
Compare
jszymanski-rtbh
commented
Jul 3, 2026
| fwrite(STDERR, "Cannot locate vendor/autoload.php. Run composer install.\n"); | ||
| exit(1); | ||
| } | ||
| require $autoload; |
Author
There was a problem hiding this comment.
Tutaj ścieżka do autoload.php różni się w zależności od instalacji: jako dependency vs jako projekt lokalny
Contributor
There was a problem hiding this comment.
Warto zatem to skomentować w pliku
peku33
reviewed
Jul 22, 2026
| $info = $api->getUserInfo(); | ||
|
|
||
|
|
||
| Cookie (username / password) Authentication |
| Authentication methods | ||
| ---------------------- | ||
|
|
||
| The SDK supports several authentication methods. ``ReportsApiSession`` accepts a single ``Auth`` object. You can choose from `ApiTokenAuth`, `DynamicApiTokenAuth` or `CookieAuth`. |
Contributor
There was a problem hiding this comment.
DynamicApiTokenAuth to typ abstrakcyjny, nie można go wybrać. Dopiero jego implementuje np. ApiTokenManager (który nota bene nie jest tu opisany bezpośrednio). To samo jeśli chodzi o CHANGELOG.md
| @@ -1,3 +1,41 @@ | |||
| # v9.0.0 | |||
| [breaking change] `ReportsApiSession` constructor now takes a single `Auth` object instead of `($username, $password)`. You can choose from `ApiTokenAuth`, `DynamicApiTokenAuth` or `CookieAuth`(old username&password method). | |||
Contributor
There was a problem hiding this comment.
Pamiętaj o temacie podbicia paczek i samego php, i oczywiście zamieszczenia tu informacji
| $storage = new JsonFileApiTokenStorage($this->path); | ||
| $apiToken = $this->apiToken; | ||
| $storage->acquireForSave(function () use ($storage, $apiToken) { | ||
| $storage->save($apiToken); |
Contributor
There was a problem hiding this comment.
W jaki sposób tu system plików jest mockowany? Czy to nam jednak pisze po oryginalnym systemie?
| return new InMemoryApiTokenStorage(new ApiToken(self::TOKEN, new \DateTimeImmutable($expiresAt))); | ||
| } | ||
|
|
||
| // configre() tests |
| public function acquireForSave(callable $callback) | ||
| { | ||
| $factory = new LockFactory(new FlockStore(dirname($this->path))); | ||
| $lock = $factory->createLock(basename($this->path), 60); |
|
|
||
| // Drop any cached token so reads inside the lock come from disk (e.g. the | ||
| // manager's re-check after another process may have rotated the token). | ||
| $this->cache = null; |
Contributor
There was a problem hiding this comment.
na pewno? a jak to zrobiliśmy w pythonie?
| // Get ApiToken from cache if set | ||
| if ($this->cache !== null) { | ||
| [$apiToken, $cachedAt] = $this->cache; | ||
| if ((microtime(true) - $cachedAt) < self::CACHE_TTL_SECONDS) { |
Contributor
There was a problem hiding this comment.
czasami używamy datetime, a czasami plain timestampów?
| throw new ApiTokenStorageException("Cannot write API token file {$this->path}: " . $exception->getMessage()); | ||
| } | ||
|
|
||
| $this->cache = null; |
Comment on lines
+13
to
+15
| "symfony/console": "^5.4", | ||
| "symfony/filesystem": "^5.4", | ||
| "symfony/lock": "^5.4" |
Contributor
There was a problem hiding this comment.
Warto sprawdzić na ile zależności się to przełożyło
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.