-
Notifications
You must be signed in to change notification settings - Fork 28
Fixed #288: allow guest-acces, disallow updates, require app users only #298
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be read from Roxy properties instead of concatenated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably, but sounds like a lot of complexity. And still not convinced Node should depend on Roxy configs. I'd prefer the other way round..
Keep in mind this piece of code is only executed at start of project, directly after executing Roxy new, so naming of default-user is reliable at that point. If you would want to make it fully dynamic, gulp serve and service scripts would have to read Roxy props too..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Safe for slush generator, as the line is commented out in build.properties by default, but worth considering for the (new) gulp init-local/dev/prod..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leveraging the relatively new ml env info --format=json in gulpfile, but overkill for slushfile..
|
I ran an elaborate test myself, but could use a test run from someone else to confirm. Had to streamline quite a bit.. |
| ' <privilege-name>rest-writer</privilege-name>\n' + | ||
| ' </privilege>\n' + | ||
| ' </privileges>\n'); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using rest-reader/writer privs also requires making sure REST extensions and such get app-role permissions, or otherwise calls those REST extensions will fail. This means patching app_specific with something like:
alias_method :original_deploy_rest, :deploy_rest
def deploy_rest
original_deploy_rest
r = execute_query(%Q{
xquery version "1.0-ml";
for $uri in cts:uris()
return (
$uri,
xdmp:document-set-permissions($uri, (
xdmp:permission("#{@properties["ml.app-name"]}-role", "read"),
xdmp:permission("#{@properties["ml.app-name"]}-role", "execute")
))
)
},
{ :db_name => @properties["ml.modules-db"] }
)
r.body = parse_json r.body
logger.debug r.body
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decided to include a pre-made app_specific.rb in templates to solve this. Showcases some other nice gimmicks as well..
0cfa613 to
f96730a
Compare
|
Ran elaborate tests with this, merging now.. |
|
This also fixed #415.. |
|
This also fixes #246.. |
|
This also fixes #240.. |
#288
Uncomment few lines in node-app.js, and you by-pass auth entirely. Streamlined default app-user stuff to make it work out of the box as Guest with rest-reader/writer privs..