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

sso_auth: fix action tag parsing #227

Merged
merged 1 commit into from
Jul 23, 2019
Merged

Conversation

Jusshersmith
Copy link
Contributor

Problem

Now that we include a Provider Slug in URL paths, the logic to parse and extract the ‘action’ from the URL incorrectly returns unknown for all (most) URL’s.

Solution

Expecting a URL path of /providerSlug/redeem for example, split the path out based on / and test with the last slice item. This manually adds back the / (as the split removes it) in order to maintain a notion of URL paths within the overall logic.

Notes

Our tests didn’t catch this - would be good to come up with a test that would have caught this.

@codecov
Copy link

codecov bot commented Jul 16, 2019

Codecov Report

Merging #227 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #227      +/-   ##
==========================================
+ Coverage   62.23%   62.25%   +0.01%     
==========================================
  Files          50       50              
  Lines        4067     4069       +2     
==========================================
+ Hits         2531     2533       +2     
  Misses       1349     1349              
  Partials      187      187
Impacted Files Coverage Δ
internal/auth/metrics.go 94.73% <100%> (+0.29%) ⬆️

internal/auth/metrics.go Outdated Show resolved Hide resolved
jphines
jphines previously approved these changes Jul 17, 2019
@Jusshersmith Jusshersmith merged commit c5f729c into master Jul 23, 2019
@Jusshersmith Jusshersmith deleted the jusshersmith-fix-action-tags branch July 23, 2019 12:26
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.

2 participants