-
Notifications
You must be signed in to change notification settings - Fork 124
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
authn-jwt
jwks-uri
with ca-cert
integration tests
#2452
Conversation
And the HTTP response content type is "application/json" | ||
And the authenticator status check succeeds | ||
|
||
Scenario Outline: ONYX-15313/6: Self-signed jwks-uri with ca-cert contains bundle includes the valid certificate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why ONYX-15313/6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This outline combines two similar tests one for self-signed use case and one for chained
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
2fa00fd
to
092548a
Compare
authn-jwt
jwks-uri
with ca-cert
integration testsauthn-jwt
jwks-uri
with ca-cert
integration tests
092548a
to
ce063c0
Compare
The server name exposes HTTPS endpoint with static chained certificate
Skip tests that's waiting for feature implementation
ce063c0
to
87f41ea
Compare
@@ -0,0 +1,14 @@ | |||
Given(/^I fetch root certificate from https:\/\/([^"]*) endpoint as "([^"]*)"$/) do |hostname, key| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use %r
around regular expression.
fetch_and_store_root_certificate(hostname: hostname, key: key) | ||
end | ||
|
||
Given(/^I successfully set authn\-jwt "([^"]*)" variable value to the "([^"]*)" certificate$/) do |variable, key| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant escape inside regexp literal
@certs ||= {} | ||
end | ||
|
||
def get_certificate_chain(connect_hostname) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CertsHelper#get_certificate_chain doesn't depend on instance state (maybe move it to another class?)
|
||
```bash | ||
cp ./certificates/nodes/chained.mycompany.local/chained.mycompany.local.cert.pem ./ | ||
cp ./certificates/nodes/chained.mycompany.local/chained.mycompany.local.key.pem ./ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line length
full chain bundle | ||
|
||
```bash | ||
cp ./certificates/nodes/chained.mycompany.local/chained.mycompany.local.cert.pem ./ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line length
Code Climate has analyzed commit 87f41ea and detected 5 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 90.9% (0.0% change). View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests approved
Desired Outcome
Test recently added
ca-cert
variable ofauthn-jwt
Implemented Changes
mycompany.local
that's using the certificate chainjwks
server is already using self signedConnected Issue/Story
ONYX-15511
Definition of Done
Changelog
CHANGELOG update
Test coverage
changes, or
Documentation
README
s) were updated in this PRBehavior
Security