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

feat: Authenticate with client certificate for /apidoc/** routes #1568

Merged
merged 14 commits into from Jun 30, 2021

Conversation

CarsonCook
Copy link
Contributor

Description

Allow certificate to be used to authenticate to /apidoc/** routes on the API Catalog.

Linked to #1523

Type of change

Please delete options that are not relevant.

  • (fix) Bug fix (non-breaking change which fixes an issue)
  • (feat) New feature (non-breaking change which adds functionality)
  • (docs) Change in a documentation
  • (refactor) Refactor the code
  • (chore) Chore, repository cleanup, updates the dependencies.
  • (BREAKING CHANGE or !) Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

For more details about how should the code look like read the Contributing guideline

Signed-off-by: Carson Cook <carson.cook@ibm.com>
Signed-off-by: Carson Cook <carson.cook@ibm.com>
Signed-off-by: Carson Cook <carson.cook@ibm.com>
Signed-off-by: Carson Cook <carson.cook@ibm.com>
Signed-off-by: Carson Cook <carson.cook@ibm.com>
Signed-off-by: Carson Cook <carson.cook@ibm.com>
@CarsonCook CarsonCook requested a review from jandadav June 23, 2021 17:19
@zowe-robot zowe-robot added the Sensitive Sensitive change that requires peer review label Jun 23, 2021
Signed-off-by: Carson Cook <carson.cook@ibm.com>
Copy link
Contributor

@jandadav jandadav left a comment

Choose a reason for hiding this comment

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

Minor comments, otherwise looks good.
There is this point:
Note: Since some services might be already providing a client certificate that is ignored during authentication, the solution needs to take this into account and preserve current behavior in cases when user credentials are provided together with the client certificate.

I wonder whether we don't need a test to prove this?

Signed-off-by: Carson Cook <carson.cook@ibm.com>
Signed-off-by: Carson Cook <carson.cook@ibm.com>
@CarsonCook CarsonCook requested a review from jandadav June 24, 2021 15:06
@CarsonCook
Copy link
Contributor Author

Minor comments, otherwise looks good.
There is this point:
Note: Since some services might be already providing a client certificate that is ignored during authentication, the solution needs to take this into account and preserve current behavior in cases when user credentials are provided together with the client certificate.

I wonder whether we don't need a test to prove this?

Added integration tests for cert + basic auth for GW URL to apidoc and not-apidoc route, as well as for service URL to apidoc route.

@jandadav
Copy link
Contributor

jandadav commented Jun 29, 2021

I am looking into the failing test, as I can run container setup and hit the same problem.

Edit: Through debug, it doesn't seem that the x509 filter in Catalog filter chain sees any certificate in request under javax.servlet.request.X509Certificate attribute. I'll keep on digging.

Signed-off-by: jandadav <janda.david@gmail.com>
@sonarcloud
Copy link

sonarcloud bot commented Jun 30, 2021

Copy link
Contributor

@jandadav jandadav 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!

@jandadav jandadav merged commit 79dedfd into master Jun 30, 2021
@delete-merged-branch delete-merged-branch bot deleted the apiml/GH1523/catalog-apidoc-with-cert branch June 30, 2021 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Sensitive Sensitive change that requires peer review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants