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: add validate oidc token call to zaas client #3897

Merged
merged 10 commits into from
Nov 13, 2024

Conversation

pablocarle
Copy link
Contributor

@pablocarle pablocarle commented Nov 12, 2024

Description

Add method in ZaasClient to use /oidc/validate endpoint in API Mediation Layer.

This implementation targets v3 Zowe, currently supporting only a boolean response (token is a valid OIDC token issued by the target API Mediation Layer's configured OIDC provider)

Type of change

  • feat: New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the style guidelines of this project
  • PR title conforms to commit message guideline ## Commit Message Structure Guideline
  • I have commented my code, particularly in hard-to-understand areas. In JS I did provide JSDoc
  • 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
  • New and existing unit tests pass locally with my changes
  • The java tests in the area I was working on leverage @nested annotations
  • Any dependent changes have been merged and published in downstream modules

zowe-robot and others added 4 commits November 12, 2024 15:49
Signed-off-by: Pablo Hernán Carle <[email protected]>
Signed-off-by: Pablo Hernán Carle <[email protected]>
Signed-off-by: Pablo Hernán Carle <[email protected]>
@pablocarle pablocarle force-pushed the reboot/feat/validate-oidc-zaasclient branch from 1b6a635 to 5531a3e Compare November 12, 2024 14:49
Copy link
Member

@achmelo achmelo left a comment

Choose a reason for hiding this comment

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

it would be great to have some integration test

@pablocarle
Copy link
Contributor Author

it would be great to have some integration test

Thanks, added

Copy link

sonarcloud bot commented Nov 13, 2024

@pablocarle pablocarle merged commit 3f0ac10 into v3.x.x Nov 13, 2024
27 checks passed
@pablocarle pablocarle deleted the reboot/feat/validate-oidc-zaasclient branch November 13, 2024 14:36
*
* This method supports simple boolean validation against Zowe v2 and v2 as of v3.0
*
* A successful validation means the token was created using the same settings as found in the target API Mediation Layer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this is non-standard for Java API doc, but should it be explicitly stated that "successful validation" means that ZaasOidcValidationResults.isValid() is true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be added to the javadoc description, no problem with that

* This method supports simple boolean validation against Zowe v2 and v2 as of v3.0
*
* A successful validation means the token was created using the same settings as found in the target API Mediation Layer.
* This version does not validate if the OIDC token is mapped to a user if the API Mediation Layer is running on the z platform.
Copy link
Contributor

@dkelosky dkelosky Nov 13, 2024

Choose a reason for hiding this comment

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

What does this mean exactly? If a token is mapped to a mainframe user, isValid() will be false? Does a service still have access to the OIDC token if the user is mapped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I wanted to clarify with this is the fact that currently the API ML will say:

  • isValid(): true if the OIDC token queried by the Zaas Client was issued by the OIDC provider that is configured in the queried API ML instance. The current implementation in the server side doesn't check if the token is mapped.
  • isValid(): false if the OIDC token is not generated by the OIDC provider or if API ML is not enabled to handle OIDC tokens.

Regarding whether a service has access to the OIDC token if the user is mapped or not, I think it's unrelated to this change, as this simply enables services using the ZaasClient to query. So it will depend on the service's setup.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it thanks.

I think it's unrelated to this change, as this simply enables services using the ZaasClient to query.

It is unrelated. I was merely curious.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

4 participants