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

fix: authorization token propagation (#1306) #1306

Merged
merged 1 commit into from
Aug 19, 2021

Conversation

tkrop
Copy link
Member

@tkrop tkrop commented Aug 19, 2021

#1306

Fix the forwarding of the authorization propagation URLs patterns during instantiation of the DefaultContextFactory.

@tkrop tkrop requested a review from zeitlinger as a code owner August 19, 2021 08:04
@codecov-commenter
Copy link

codecov-commenter commented Aug 19, 2021

Codecov Report

Merging #1306 (ca8e034) into master (24156b8) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1306   +/-   ##
=========================================
  Coverage     76.02%   76.02%           
  Complexity     1017     1017           
=========================================
  Files           187      187           
  Lines          2937     2937           
  Branches        508      508           
=========================================
  Hits           2233     2233           
  Misses          435      435           
  Partials        269      269           
Impacted Files Coverage Δ
...zally/configuration/ContextFactoryConfiguration.kt 100.00% <100.00%> (ø)

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 24156b8...ca8e034. Read the comment docs.

@tkrop
Copy link
Member Author

tkrop commented Aug 19, 2021

👍

Copy link
Contributor

@vadeg vadeg left a comment

Choose a reason for hiding this comment

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

Thank you!

I would simplify a little bit to reduce lateinit noise:

class ContextFactoryConfiguration {

    @Bean
    fun contextFactory(@Value("\${zally.propagateAuthorizationUrls:}") propagateAuthorizationUrls: List<Pattern>): DefaultContextFactory {
        return DefaultContextFactory(propagateAuthorizationUrls)
    }
}

@tkrop
Copy link
Member Author

tkrop commented Aug 19, 2021

@vadeg LOL I would have left the original version before the refactoring that broke it to keep the code size even smaller (see 6d9b7aa#diff-1247b8e09fafc4c737a084e1d5efbf7840a79389e82d01e1b2d1fb25c376f00cL20). But for now lets go with the pull request to test this.

@vadeg
Copy link
Contributor

vadeg commented Aug 19, 2021

@tkrop I have tested this already.

@vadeg
Copy link
Contributor

vadeg commented Aug 19, 2021

👍

@tkrop tkrop merged commit 4f83eb3 into master Aug 19, 2021
@tkrop tkrop deleted the fix-authorization-propagation branch August 19, 2021 10:33
@tkrop tkrop self-assigned this Aug 26, 2021
@tkrop tkrop added area: server Zally server issues type: bug labels Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: server Zally server issues type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants