This repository has been archived by the owner on Nov 9, 2022. It is now read-only.
Issue 563 #674
Merged
Issue 563 #674
Conversation
This file contains 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
Add <uri-param name="language">sjs</uri-param> to your request definition to make use of a SJS controller. Issue marklogic-community#640.
…they are required for Roxy's rewriter
Updated previous changes based on dcassel's suggestions. Tested using the sample SJS requests and test.sjs in marklogic-community#640.
marklogic-community#640: Support SJS with a language property
marklogic-community#640 updating unit tests for SJS controllers
|
Granting any-uri could definitely circumvent security at larger orgs and gov. Preferably could we just define the proper URI permissions? That being said, I'm also totally cool with merging this and then defining this task in a separate issue for a later milestone. Beyond that, it looks good to me! |
Conflicts: src/test/suites/Framework Tests/routing.xqy src/test/suites/Framework Tests/site-index.xqy
The amps approach doesn't work, because we support using the filesystem for modules. Amps may not be applied to filesystem code unless it's under the MarkLogic install directory. New approach: the testing role gets the general app role, plus several privileges needed for running tests.
They don't work because they rely on property substitution in test-config.xqy to indicate what user the requests should be made as.
| @@ -458,6 +458,12 @@ | |||
| </collections> | |||
| <privileges> | |||
| <privilege> | |||
| <privilege-name>any-uri</privilege-name> | |||
There was a problem hiding this comment.
Doesn't this any-uri assignment belong in the (optional) ${app-role}-unit-test ?
There was a problem hiding this comment.
Ack. Yes, it does. I'll move that and run another comparison between the ml-config.xml I tested with and this file.
|
@RobertSzkutak I moved the any-uri privilege. |
|
That looks good! It worked fine last night when I tested it too. Merging.. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR makes unit tests work out of the box with the default user (ie, no longer requiring admin for tests to work).
Question for reviewers: any concerns with the privileges that I've granted to the default user?
Three of those are required for Roxy's rewriter to work (see src/roxy/rewriter-lib.xqy rewrite-rules();
eput:get-rest-optionsrequires rest-reader).The amps should be safe, in that the code they refer to isn't even deployed in production.