Skip to content

Conversation

@syjer
Copy link
Contributor

@syjer syjer commented Aug 11, 2020

Hi @danfickle this PR is an initial work as discussed in #508 for adding some kind of context when checking the access of external resource.

I've added some stubs for defining the new interfaces. It should be retro compatible. The objective is, for example in the case of #508, to call the more precise getResourceXYZ(uri, type) instead of the generic one.

WDYT? :)

@syjer syjer marked this pull request as draft August 11, 2020 10:48
@danfickle
Copy link
Owner

Good so far. The question will be the builder api. Do you think the user provides one lambda for all resource types? Or can they supply a lambda per resource type and we do the filtering? I think the first one is preferable as it lets the user ban for example file protocol uris with minimal fuss. I could be convinced though. We also have to think carefully about the default lambda implementation so as not to break anything while keeping maximum security.

@syjer
Copy link
Contributor Author

syjer commented Aug 11, 2020

I think allowing both style is not a bad idea. For example if a user would like to filter only for the file attachment case, allowFor(ExternalResourceType, Predicate<String>); is less error prone than changing the default lambda.

We also have to think carefully about the default lambda implementation so as not to break anything while keeping maximum security.

Agree. I think currently we can allow everything (as it is now) except the "file attachment" usecase in #508 .

@danfickle
Copy link
Owner

I think allowing both style is not a bad idea.

Agreed, from a user point of view. May be a little bit of work to get right on this end though.

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.

2 participants