Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tenant identification in assets when using path-identification #1121

Closed
wants to merge 6 commits into from

Conversation

craigrileyuk
Copy link

By default, calling tenant_asset() results in an asset URL like '/tenancy/assets/{path}`. This works fine for everything except path-identification since the tenant_id is required to be in the path itself.

These fixes add a second parameter to tenant_asset(string $path, bool $usePathIdentification = false) which uses a different route to enable proper URLs can be generated.

This restored functionality required a few other tweaks and some extra protection against path traversal attacks.

@zscalersptest
Copy link

comment

@stancl
Copy link
Member

stancl commented Aug 24, 2023

Hi, thanks for the PR. We're dealing with improving path identification and the asset helper in v4 now, so I went over the code here in depth and I think we'll be handling things differently.

Regarding path traversal: this doesn't seem to be an issue on modern webservers, but it might be worth adding that check just in case. I'll commit this in a modified way to the 3.x branch.

Regarding the duplicated route and forgetParameter(), we have a different way of doing that in v4, so we'll address it there.

@stancl stancl closed this Aug 24, 2023
@stancl
Copy link
Member

stancl commented Aug 24, 2023

4af70d3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants