8000 feat: support regular raw acl rule in jwt acl claim by zmstone · Pull Request #12189 · emqx/emqx · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@zmstone
Copy link
Member
@zmstone zmstone commented Dec 17, 2023

Fixes #12187 and EMQX-11372
Release version: v/e5.5.0

Summary

PR Checklist

Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:

  • Added tests for the changes
  • Added property-based tests for code which performs user input validation
  • Changed lines covered in coverage report
  • Change log has been added to changes/(ce|ee)/(feat|perf|fix|breaking)-<PR-id>.en.md files
  • For internal contributor: there is a jira ticket to track this change
  • Created PR to emqx-docs if documentation update is required, or link to a follow-up jira ticket
  • Schema changes are backward compatible

Checklist for CI (.github/workflows) changes

  • If changed package build workflow, pass this action (manual trigger)
  • Change log has been added to changes/ dir for user-facing artifacts update

@zmstone zmstone force-pushed the 1217-feat-jwt-authz-support-ignore-action branch 6 times, most recently from b87fdb8 to cdc3e5f Compare December 18, 2023 20:47
@zmstone zmstone marked this pull request as ready for review December 18, 2023 20:48
@zmstone zmstone requested review from a team, JimMoen and savonarola as code owners December 18, 2023 20:48
@zmstone zmstone force-pushed the 1217-feat-jwt-authz-support-ignore-action branch 4 times, most recently from dbf8029 to 93651e8 Compare December 18, 2023 21:03
@zmstone zmstone force-pushed the 1217-feat-jwt-authz-support-ignore-action branch from 93651e8 to 2be898c Compare December 19, 2023 07:10
catch
throw:{bad_acl_rule, Reason} ->
%% it's a invalid token, so ok to log
?TRACE_AUTHN_PROVIDER("bad_acl_rule", Reason#{jwt => JWT}),
Copy link
8000 Contributor

Choose a reason for hiding this comment

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

Previously, the validation was lazy, that is, invalid rules were handled on authz stage. This is kept for v1 rules;
For v2 we now break connection, don't we consider this to be a breaking change?

Copy link
Member Author

Choose a reason for hiding this comment

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

A invalid token will cause any pub/sub action to fail, maybe it's better to close connection early.
It's a new format, so maybe it's not entirely a breaking change since all the v1 clients will not notice any change.

JWT = generate_jws(Payload),
test_topic_rules(JWT).

t_topic_rules_v2(_Config) ->
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe add qos/retain to v2 examples

@zmstone zmstone merged commit 1894349 into emqx:master Dec 19, 2023
@zmstone zmstone deleted the 1217-feat-jwt-authz-support-ignore-action branch December 19, 2023 10:34
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.

allow jwt acl to passthrough

2 participants

0