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

Introduce Configuration Pattern for Authn Handlers and Scopes #15363

Merged
merged 12 commits into from
May 29, 2019

Conversation

ldclakmal
Copy link
Member

Purpose

This is an improvement for Ballerina inbound authentication discussed at [1]. With this improvement, authn handlers and scopes can be engaged with both OR, AND logic. Example for scope engagement is as follows:

  1. Auth should be successful for scope-1 OR scope-2.
    scopes: ["scopes-1", "scopes-2"]

  2. Auth should be successful for scope-1 AND scope-2.
    scopes: [["scopes-1"], ["scopes-2"]]

  3. Auth should be successful for ((scope-1 OR scope-2) AND (scope-3 OR scope-4)).
    scopes: [["scopes-1", "scopes-2"], ["scopes-3", "scopes-4"]]

Also, the same configuration pattern can be applied for auth providers as well.

[1] https://groups.google.com/d/msg/ballerina-dev/U3-GY9Q49eQ/TFUaJeZkAwAJ

Remarks

Related PR - #15056

Check List

  • Read the Contributing Guide
  • Required Balo version update
  • Updated Change Log
  • Checked Tooling Support (#)
  • Added necessary tests
    • Unit Tests
    • Spec Conformance Tests
    • Integration Tests
    • Ballerina By Example Tests
  • Increased Test Coverage
  • Added necessary documentation
    • API documentation
    • Module documentation in Module.md files
    • Ballerina By Examples

stdlib/http/src/main/ballerina/http/annotation.bal Outdated Show resolved Hide resolved
stdlib/http/src/main/ballerina/http/auth/authz_filter.bal Outdated Show resolved Hide resolved
stdlib/http/src/main/ballerina/http/auth/authn_filter.bal Outdated Show resolved Hide resolved
(boolean|error?)[] responseArray = [];
int count = 0;
foreach AuthnHandler?[] authnHandler in authnHandlers {
responseArray[count] = checkForAuthnHandlers(authnHandler, request);
Copy link
Member

Choose a reason for hiding this comment

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

Can we break early, without us having to go through all the authentication.

Since we are evaluating AND in this situation (array of arrays), if any of the group was evaluated to be "false" there is no need to proceed invoking other authenticators.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. Could be improved without the use of the responseArray. Done with 3ed5350

@codecov-io
Copy link

codecov-io commented May 21, 2019

Codecov Report

Merging #15363 into master will increase coverage by 0.74%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #15363      +/-   ##
============================================
+ Coverage     22.18%   22.93%   +0.74%     
+ Complexity     6136     5112    -1024     
============================================
  Files          1798     1346     -452     
  Lines         83671    69308   -14363     
  Branches      12085     9384    -2701     
============================================
- Hits          18563    15894    -2669     
+ Misses        62095    51636   -10459     
+ Partials       3013     1778    -1235
Impacted Files Coverage Δ Complexity Δ
...va/org/ballerinalang/stdlib/task/service/Init.java 52.63% <0%> (-47.37%) 8% <0%> (ø)
...ang/stdlib/task/objects/ServiceWithParameters.java 53.57% <0%> (-46.43%) 2% <0%> (-3%)
...lang/stdlib/task/utils/ResourceFunctionHolder.java 40% <0%> (-45.72%) 2% <0%> (ø)
...rg/ballerinalang/stdlib/task/service/Register.java 60% <0%> (-40%) 3% <0%> (ø)
...lib/runtime/nativeimpl/InvocationContextUtils.java 50.87% <0%> (-39.75%) 8% <0%> (ø)
.../ballerinalang/stdlib/task/utils/TaskExecutor.java 45.83% <0%> (-38.79%) 3% <0%> (ø)
...ava/org/ballerinalang/stdlib/task/utils/Utils.java 43.83% <0%> (-38.22%) 14% <0%> (ø)
...inalang/stdlib/file/service/endpoint/Register.java 48.71% <0%> (-37.65%) 9% <0%> (ø)
...lang/stdlib/crypto/nativeimpl/DecodePublicKey.java 42.05% <0%> (-36.9%) 2% <0%> (ø)
...lang/stdlib/task/impl/TaskServerConnectorImpl.java 64.28% <0%> (-35.72%) 2% <0%> (-1%)
... and 516 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad82f36...ac17715. Read the comment docs.

@ldclakmal ldclakmal added Team/StandardLibs All Ballerina standard libraries Type/Improvement labels May 22, 2019
@ldclakmal ldclakmal added this to the 0.995.0 milestone May 22, 2019
@ldclakmal ldclakmal requested a review from ayomawdb May 22, 2019 09:16
…ng/test/auth/AuthzConfigPatternTest.java

Co-Authored-By: Gihan Anuruddha <wggihan@users.noreply.github.com>
@ldclakmal ldclakmal merged commit 5fdf8bb into ballerina-platform:master May 29, 2019
@ldclakmal ldclakmal deleted the improve-inbound-auth branch March 27, 2020 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team/StandardLibs All Ballerina standard libraries Type/Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants