fix: answer a created public link with the hash that is its URL - #820
Merged
Conversation
`account/viewLink/{hash}` is the URL a public link hands out, and the service
mints the hash — so an API caller creating a link got back only what they had
sent, with `hash: null`, and could not hand out what they had just made.
The way round it was to fetch the link again with `publicLink/view`, which
answers with `data` as well — the sealed vault — so working around the omission
gave out more than reporting the hash does.
It costs nothing: the controller already reads the stored link back for the
expiry and the view limit it had been misreporting, so this is one more field
off an object already in hand. `Account\CreateController` and
`AuthToken\CreateController` both read their record back for the same reason.
`data` stays out. Returning the whole stored model would have been the shorter
change and would have handed the sealed payload to a token scoped only to
PUBLICLINK_CREATE, so the test asserts `data` is null alongside asserting the
hash is present, and the response cannot quietly widen later.
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
1 participant
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.
account/viewLink/{hash}is the URL a public link hands out, and the service mints the hash — so an API caller creating a link got back only what they had sent, withhash: null, and could not hand out what they had just made.The way round it was to fetch the link again with
publicLink/view. That answers withdataas well — the sealed vault — so working around the omission gave out more than reporting the hash does.Account\CreateControllerandAuthToken\CreateControllerboth already read the stored record back for this reason; the auth-token one carries a note about it, since a token a caller cannot read is a token that was not created. This is the same shape.Cost
None. #812 already made this controller read the stored link back, for the expiry and the view limit it had been misreporting. This is one more field off an object already in hand.
What is deliberately still absent
data— the sealed vault — stays out of the response. Returning the whole stored model would have been the shorter change and would have handed the sealed payload to a token scoped only toPUBLICLINK_CREATE. The test assertsdatais null as well as asserting the hash is present, so the response cannot quietly widen later.Test
One test, asserting both directions: the hash comes back, it is the one the row is stored under, and
datadoes not. Removing the field fails it withFailed asserting that a NULL is not empty.3967 unit tests pass; PHPStan level 6 on
srcand PHPCS clean.