Skip to content

Conversation

lodrantl
Copy link
Contributor

Resolves #26.

@lodrantl lodrantl changed the title Bearer only Bearer only client Dec 20, 2017
@Trojan295
Copy link
Contributor

Trojan295 commented Jan 15, 2018

@lodrantl, could you rebase the PR from the current master? There was a problem with CI, which is fixed now.

@coveralls
Copy link

coveralls commented Feb 2, 2018

Pull Request Test Coverage Report for Build 63

  • 14 of 14 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.5%) to 55.318%

Files with Coverage Reduction New Missed Lines %
./test/unit/mockable_case.lua 1 97.83%
Totals Coverage Status
Change from base Build 60: 0.5%
Covered Lines: 1061
Relevant Lines: 1918

💛 - Coveralls

@lodrantl
Copy link
Contributor Author

lodrantl commented Feb 2, 2018

Finnally took the time to rebase and add the tests. Also deployed it to staging env and we'll see how it goes.

@tsyrjanen
Copy link
Collaborator

Looks good. @phirvone could you also look at this.

local res, err = require("resty.openidc").introspect(oidcConfig)
if err then
if oidcConfig.bearer_only == "yes" then
ngx.header["WWW-Authenticate"] = 'Bearer realm="kong",error="' .. err .. '"'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should realm here be hardcoded? IDP can have different realms and I think it should be also a configuration option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also thought about that, but didn't want to overcomplicate the configuration. But now that I think again, I am all for a configurable realm with a sane default.

@Trojan295 Trojan295 requested a review from phirvone February 18, 2018 21:10
@Trojan295 Trojan295 merged commit 029a7e9 into nokia:master Feb 20, 2018
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.

4 participants