feat: 79 standard login should be supported#155
Conversation
krowvin
left a comment
There was a problem hiding this comment.
Few comments I had to the reviewers
|
|
||
| - Override the client ID, OIDC base URL, and scopes: | ||
|
|
||
| ``cwms-cli login --client-id cwms --oidc-base-url https://identity-test.cwbi.us/auth/realms/cwbi/protocol/openid-connect --scope "openid profile"`` |
There was a problem hiding this comment.
this will publish this and make it static, a thing for maintenance?
There was a problem hiding this comment.
maybe just a very generic placeholder URL? The actual URL for for the login should be derived from the CDA target OpenAPI spec.
|
Initial PR of these changes requested to @Enovotny and/or @MikeNeilson Will continue the second half of this PR as it currently only saves the token to disk. Need HydrologicEngineeringCenter/cwms-python#274 to make use of the token that is saved. |
|
If you would like to test this locally: pip install --upgrade git+https://github.com/HydrologicEngineeringCenter/cwms-cli.git@79-standard-login-should-be-supportedthen cwms-cli login |
MikeNeilson
left a comment
There was a problem hiding this comment.
Is this just the ability to login right now? I'm not seeing how it integrates with actually calls to CDA - just want to verify if that is intentional right now.
|
|
||
| - Override the client ID, OIDC base URL, and scopes: | ||
|
|
||
| ``cwms-cli login --client-id cwms --oidc-base-url https://identity-test.cwbi.us/auth/realms/cwbi/protocol/openid-connect --scope "openid profile"`` |
There was a problem hiding this comment.
maybe just a very generic placeholder URL? The actual URL for for the login should be derived from the CDA target OpenAPI spec.
@MikeNeilson that is correct via #155 (comment) I have the issue made here and i'm working on PR that for cwms-python. But if this were to change drastically it could change that PR so asking for initial review. Will request again when I mark it as "ready for review". |
|
Then we could remove the workaround mentioned above Need to test this more before I mark it ready, submitting here for visibility |
What's off about the URL? at least dev and test should be using the not identityc version now. Can't remember for prod. |
I was having some trouble getting the URL from the swagger spec, but I submitted a PR that enabled me to do so easier |
Alters CDA published OpenID Connect metadata and improves local OIDC
test coverage
Changes:
- remove the invalid HTTP auth scheme("openid") from the oIdConnect
security scheme so Swagger publishes a correct oIdConnectUrl
- add a focused test for openID security scheme
- add debug logging for JWT validation failures to expose the underlying
claim/signature issue during local auth setup
- update local compose seed data so m5hectest has TS ID Creator in SWT,
matching the documented/test write-user expectations
Used this in testing
- HydrologicEngineeringCenter/cwms-cli#155
# Conflicts: # docs/cli/blob.rst # docs/index.rst # tests/commands/test_blob_upload.py
…uld-be-supported # Conflicts: # cwmscli/load/timeseries/timeseries_ids.py
|
I tried this on localhost and was able to use Might take a final peak @MikeNeilson before we merge this in? |
…-resolve # Conflicts: # cwmscli/commands/blob.py # cwmscli/commands/commands_cwms.py # cwmscli/commands/shef/import_critfile.py
🤖 I have created a release *beep* *boop* --- ## [0.6.0](v0.5.0...v0.6.0) (2026-04-28) ### Features * 79 standard login should be supported ([#155](#155)) ([35d0180](35d0180)) * Add shef in file export, update shef command ([#196](#196)) ([4b3e0bf](4b3e0bf)) ### Bug Fixes * 158 add smart complete feature to the cli ([#178](#178)) ([e755cc9](e755cc9)) * Fix blob-id path for delete ([#201](#201)) ([bbfb87d](bbfb87d)) * Update ini import ([#212](#212)) ([b02eb8d](b02eb8d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Drafting this as I work through it for visibility
Created issue with cwms-python here:
HydrologicEngineeringCenter/cwms-python#274
Once that has been merged in, will update this PR to include those changes and make it so this login method can be used for cli commands.