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

Support APIML login with PEM certificates #2802

Merged
merged 34 commits into from
May 8, 2024

Conversation

t1m0thyj
Copy link
Member

@t1m0thyj t1m0thyj commented Mar 25, 2024

Proposed changes

Resolves #2621 by adding prompt to choose between "User and Password" or "Certificate" when logging in to authentication service:
image

When the Certificate option is selected, then a webview is opened to browse for certificate files:
image

Thanks @awharn and @traeok for co-authoring this 🙂

Release Notes

Milestone: 2.16.0

Changelog: Added PEM certificate support as an authentication method for logging into the API ML

Types of changes

What types of changes does your code introduce to Zowe Explorer?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Updates to Documentation or Tests (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This checklist will be used as reference for both the contributor and the reviewer

  • I have read the CONTRIBUTOR GUIDANCE wiki
  • PR title follows Conventional Commits Guidelines
  • PR Description is included
  • gif or screenshot is included if visual changes are made
  • yarn workspace vscode-extension-for-zowe vscode:prepublish has been executed
  • All checks have passed (DCO, Jenkins and Code Coverage)
  • I have added unit test and it is passing
  • I have added integration test and it is passing
  • There is coverage for the code that I have added
  • I have tested it manually and there are no regressions found
  • I have added necessary documentation (if appropriate)
  • Any PR dependencies have been merged and published (if appropriate)

Further comments

t1m0thyj and others added 7 commits March 20, 2024 16:59
Co-authored-by: Andrew W. Harn <[email protected]>
Signed-off-by: Timothy Johnson <[email protected]>
Co-authored-by: Andrew W. Harn <[email protected]>
Co-authored-by: Trae Yelovich <[email protected]>
Signed-off-by: Timothy Johnson <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Co-authored-by: Andrew Harn <[email protected]>
- Added "Cancel" button to webview
- Pick up existing cert paths from service profile
- Perform login on submit and close webview
- Error handling for dismissed/cancelled webview

Signed-off-by: Trae Yelovich <[email protected]>
Co-authored-by: Andrew Harn <[email protected]>
Co-authored-by: Timothy Johnson <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Co-authored-by: Andrew Harn <[email protected]>
Co-authored-by: Timothy Johnson <[email protected]>
@t1m0thyj t1m0thyj added this to the v2.16.0 milestone Mar 25, 2024
Copy link

codecov bot commented Mar 26, 2024

Codecov Report

Attention: Patch coverage is 91.56627% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 93.46%. Comparing base (5de7967) to head (c69020c).

Files Patch % Lines
...kages/zowe-explorer/src/utils/CertificateWizard.ts 90.19% 5 Missing ⚠️
...owe-explorer-api/src/vscode/ZoweVsCodeExtension.ts 91.30% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2802   +/-   ##
=======================================
  Coverage   93.46%   93.46%           
=======================================
  Files         103      104    +1     
  Lines       10763    10841   +78     
  Branches     2345     2276   -69     
=======================================
+ Hits        10060    10133   +73     
- Misses        702      707    +5     
  Partials        1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

traeok and others added 8 commits March 26, 2024 14:03
Signed-off-by: Trae Yelovich <[email protected]>
Co-authored-by: Andrew Harn <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
- awharn wrote this test, but this was done over Live Share :)

Signed-off-by: Trae Yelovich <[email protected]>
Co-authored-by: Andrew Harn <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Co-authored-by: Timothy Johnson <[email protected]>
Co-authored-by: Andrew Harn <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
@traeok traeok marked this pull request as ready for review March 27, 2024 20:21
@JTonda JTonda requested a review from zFernand0 April 16, 2024 15:02
@traeok traeok marked this pull request as draft April 23, 2024 15:04
@traeok traeok marked this pull request as ready for review April 23, 2024 16:15
zFernand0
zFernand0 previously approved these changes Apr 30, 2024
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

LGTM! 😋
Not sure if we really need to update the webviews/changelog.md file. 😅

@JTonda JTonda requested a review from zFernand0 April 30, 2024 15:04
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

Apologies for the delay.
Functionality works great!
LGTM! 😋

Here is a GIF with the behavior working as expected through a local instance of the APIML
temp

And here are some steps that you can use to try this yourself.
Notes:

  • I recommend following the APIML prereqs for running it locally
  • Personally I suggest using Node 20 and Java 11
  • The Gradle build and APIML startup may take around 10 minutes (or more)😋
  1. git clone https://github.com/zFernand0/api-layer -b zfernand0/test/cert-login ze-cert

  2. cd ze-cert

  3. ./gradlew build -x test (skip running tests to speed up the process)

  4. npm i -g concurrently@6

  5. npm run api-layer

  6. Wait for the APIML Gateway to come up (navigate to https://localhost:10010)

  7. Modify your config file with the following details

    • "host": "localhost"
    • "port": 10010
    • "basePath": "/mockzosmf/api"
    • "rejectUnauthorized": false
    Full config? Toggle me!
     {
         "$schema": "./zowe.schema.json",
         "profiles": {
             "apiml": {
                 "type": "zosmf",
                 "properties": {
                     "basePath": "/mockzosmf/api"
                 },
                 "secure": []
             },
             "base": {
                 "type": "base",
                 "properties": {
                     "host": "localhost",
                     "port": 10010,
                     "rejectUnauthorized": false
                 },
                 "secure": []
             }
         },
         "defaults": {
             "zosmf": "apiml",
             "base": "base"
         },
         "autoStore": true
     }
    
  8. Add the apiml profile in Zowe Explorer (if not automatically added)

  9. Right-click on apiml > Manage Profile > Log In to Auth Service > Certificate

  10. Specify the <ze-cert>/keystore/client_certs/newfile.crt.pem as the certificate file

  11. Specify the <ze-cert>/keystore/client_certs/newfile.key.pem as the certificate key file

If you run into any issues, please let me know 😋

@zFernand0 zFernand0 mentioned this pull request May 8, 2024
2 tasks
Copy link

sonarqubecloud bot commented May 8, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
4.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@JillieBeanSim JillieBeanSim merged commit 7995b02 into main May 8, 2024
20 of 22 checks passed
@JillieBeanSim JillieBeanSim deleted the feat/apiml-login-certificates branch May 8, 2024 13:55
@traeok traeok linked an issue Dec 18, 2024 that may be closed by this pull request
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.

Support Certificate Authentication Implement certificate authentication support for API ML
6 participants