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

[NOID] Use Injected UrlAccessChecker for checking access to URLs #537

Merged
merged 1 commit into from
Nov 24, 2023

Conversation

mnd999
Copy link
Contributor

@mnd999 mnd999 commented Nov 16, 2023

Fixes #<Replace with the number of the issue, Mandatory>

Use Injected URLAccessChecker for checking access to URLs calling internal WebURLAccessRule directly
Proposed Changes (Mandatory)

Inject a new URLAccessChecker to separate out APOC from making internal calls into WebURLAccessRule, as this needs to changes to support RBAC permissions as well. Having this injectable will mean that we can late get this new functionality in APOC as well for free.

The alternative is to move the config only based checks into APOC.

@mnd999 mnd999 added NOT READY FOR MERGE PR isn't ready to be merged dev labels Nov 16, 2023
@CLAassistant
Copy link

CLAassistant commented Nov 16, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@loveleif loveleif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

Comment on lines +50 to +51
@Context
public URLAccessChecker urlAccessChecker;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this breaks backwards compatibility, but that's ok perhaps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it does. But that's okay, right - we require a matching APOC version with each Neo4j release? How would we usually go about merging something that breaks like this? Core first, then APOC?

@mnd999 mnd999 marked this pull request as ready for review November 23, 2023 10:38
…ead of calling internal WebURLAccessRule directly
@AzuObs AzuObs merged commit e01a29d into dev Nov 24, 2023
10 of 11 checks passed
@AzuObs AzuObs deleted the dev-use-injected-urlchecker branch November 24, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev NOT READY FOR MERGE PR isn't ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants