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

REST endpoint /rules should be accessible without admin rights #3390

Closed
hmerk opened this issue Feb 20, 2023 · 6 comments · Fixed by #3392
Closed

REST endpoint /rules should be accessible without admin rights #3390

hmerk opened this issue Feb 20, 2023 · 6 comments · Fixed by #3392

Comments

@hmerk
Copy link

hmerk commented Feb 20, 2023

With the new scenes aeditor being merged into MainUI, there is a need to extend the oh-repeater component to access rules with tags "Scene" for creating custom widgets. This is already prepared with
openhab/openhab-webui#1724

To make it work, the REST endpoint for rules needs o be accessible without admin rights.

@florian-h05
Copy link
Contributor

florian-h05 commented Feb 20, 2023

This affects the GET /rest/rules endpoint, we only need unauthorised access to it with the &summary=true request param set. This would keep the rule code „safe“. I‘d propose to therefore allow unauthorised access to this endpoint only when summary=true.

I guess the rule action is affected by this as well, it is using the POST /rest/rules/<uid>/runnow endpoint (see https://github.com/openhab/openhab-webui/blob/17f12f51843b711c30ecb0eac60649d50335f677/bundles/org.openhab.ui/web/src/components/widgets/widget-actions.js#L150).

Both these endpoints are type of security relevant, we could probably make the unauthorised access for widgets configurable, so the user can disable it if he doesn‘t need it and has security concerncs.

@hmerk
Copy link
Author

hmerk commented Feb 20, 2023

I guess the rule action is affected by this as well, it is using the POST /rest/rules/<uid>/runnow endpoint (see

I don't think so, as the rule action is not newly introduced and it would not have made sense to implement it without being able to use it within the widgets. Therefore I would assume that it is already accessible without authorization.

@florian-h05
Copy link
Contributor

I have just checked, and you are right.

@stefan-hoehn
Copy link
Contributor

stefan-hoehn commented Feb 20, 2023

I checked the rules-endpoint in RuleResource.java

if I am not mistaken the applicable methods should be

  • runNow
  • get

I think we rather have a problem on the runnow than on the getRules or does the absence of "RolesAllowed" mean something different?

@GET
    @Produces(MediaType.APPLICATION_JSON)
    @Operation(operationId = "getRules", summary = "Get available rules, optionally filtered by tags and/or prefix.", responses = {
            @ApiResponse(responseCode = "200", description = "OK", content = @Content(array = @ArraySchema(schema = @Schema(implementation = EnrichedRuleDTO.class)))) })
    public Response get(@QueryParam("prefix") final @Nullable String prefix,
  
@POST
    @RolesAllowed({ Role.USER, Role.ADMIN })
    @Path("/{ruleUID}/runnow")
    @Consumes(MediaType.TEXT_PLAIN)
    @Operation(deprecated = true, operationId = "runRuleNow", summary = "Executes actions of the rule.", responses = {
            @ApiResponse(responseCode = "200", description = "OK"),
            @ApiResponse(responseCode = "404", description = "Rule corresponding to the given UID does not found.") })
    public Response runNow(@PathParam("ruleUID") @Parameter(description = "ruleUID") String ruleUID)

but I think I read that wrong because my tests tell me something else: I cannot see the rules when I am not logged in (I get a 401 on the /rules endpoint).

@florian-h05
Copy link
Contributor

You found the right methods, but you read it wrong:

The RuleResource class is annotated with @RolesAllowed({ Role.ADMIN }), which inherits to all methods of this class (see https://docs.oracle.com/javaee/7/api/javax/annotation/security/RolesAllowed.html).
The get method and most other methods therefore don’t need the @RolesAllowed annotation to only allows admin.
The runNow method has an explicit @RolesAllowed annotation to allow this method for the user role as well.

We therefore need to add the @RolesAllowed annotation oft ADMIN and USER to the get method as well.

@stefan-hoehn
Copy link
Contributor

So, I only need to add that Role add that place. I am happy to the change it.

@J-N-K As you initially started the scene editor, what do you think about opening the roles for that endpoint like @hmerk has proposed?

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 a pull request may close this issue.

3 participants