Authorize push server, rather than asset store#64
Open
tomcoldrick-ct wants to merge 2 commits intomainfrom
Open
Authorize push server, rather than asset store#64tomcoldrick-ct wants to merge 2 commits intomainfrom
tomcoldrick-ct wants to merge 2 commits intomainfrom
Conversation
tomcoldrick-ct
force-pushed
the
coldtom/authorize-push
branch
from
2857ba2 to
526573f
Compare
Authorizing the asset store means that anything a fetcher wants to push into the store must pass the push authorizer. This isn't what we want, as fetchers should be able to push into the store with impunity. Instead we move the authorization up a layer to wrap the push server. The fetcher is already wrapped.
tomcoldrick-ct
force-pushed
the
coldtom/authorize-push
branch
from
526573f to
5fe92bb
Compare
Contributor
|
Is it worth removing the |
Collaborator
Author
Good point, I was thinking about leaving it around since it's still a fine wrapper, but actually using it would lead to the footgun reported in #62 and we don't expose any way to configure it, so I've removed it now. |
Authorization should be handled by the fetch/push servers, not the asset store.
tomcoldrick-ct
force-pushed
the
coldtom/authorize-push
branch
from
a4e2ed5 to
2f4cfba
Compare
sdclarke
approved these changes
Sep 5, 2025
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.
Authorizing the asset store means that anything a fetcher wants to push into the store must pass the push authorizer. This isn't what we want, as fetchers should be able to push into the store with impunity. Instead we move the authorization up a layer to wrap the push server. The fetcher is already wrapped.
Fixes #62