refactor basic authentication and session management

[adamkorynta]

Problem Description

All jax-rs resource endpoints include the user token parameter that is redundant with the authorization header. In order to implement CWMS CAC authentication, all basic authentication needs to get refactored so that it is not enabled in the CWMS/CCP version of the REST API.

Solution

• Removed query parameter user token. Authorization headers are now the only way to authenticate with the REST API when using basic authentication.
• Refactored token authentication to a ContainerRequestFilter in order to facilitate a separate implementation for CWMS CAC authentication.
• The token is now moved into the context session.
• There is still a thread monitoring for stale ApiEventClient and ApiLddsClients, though they are keyed to session ids so that they can get shutdown when the session is destroyed.
• removed the -s secure mode flag given that the insecure query parameters were gated by the flag and have been removed
• Added authorization for JWT and Single Sign-On JSESSIONIDSSO cookie checks.

how you tested the change

Used IntelliJ HTTP client checking authenticated vs non-authenticated endpoints. Added unit tests with heavy (probably too heavy) usage of Mockito.

Where the following done:

• [x] Tests. Check all that apply:
  • [x] Unit tests created or modified that run during gradle test.
  • [ ] Integration tests created or modified that run during integration testing (Formerly called regression tests.)
  • [ ] Test procedure descriptions for manual testing
• [x] Was relevant documentation updated? updated README.md to document the update.
• [x] Were relevant config element (e.g. XML data) updated as appropriate

Relevant discussion page for removing the query parameters: https://github.com/opendcs/rest_api/discussions/185

[MikeNeilson]

Oh, there's an additional link here: https://github.com/opendcs/rest_api/issues/18

I think we'll want to go there eventually, but this PR is a solid first step.

[MikeNeilson]

This api, except login, should probably not allow guest operations even for get.

While 7.0.13 has introduced a why to hide secrets they will still end up in various properties and allowing guest access without strong consideratio is likely to leak credentials like a seeve.

On Fri, Sep 13, 2024, 4:22 PM sonarcloud[bot] @.***> wrote:

[image: Quality Gate Failed] https://sonarcloud.io/dashboard?id=opendcs_rest_api&pullRequest=186 Quality Gate failed

Failed conditions

https://camo.githubusercontent.com/bd1d5234b37410841ad8cd9d85c84eeecaec92ae18a95c926ae3a59a17c29a65/68747470733a2f2f736f6e6172736f757263652e6769746875622e696f2f736f6e6172636c6f75642d6769746875622d7374617469632d7265736f75726365732f76322f636f6d6d6f6e2f6661696c65642d313670782e706e67 9.8% Coverage on New Code https://sonarcloud.io/component_measures?id=opendcs_rest_api&pullRequest=186&metric=new_coverage&view=list (required ≥ 80%)

https://camo.githubusercontent.com/bd1d5234b37410841ad8cd9d85c84eeecaec92ae18a95c926ae3a59a17c29a65/68747470733a2f2f736f6e6172736f757263652e6769746875622e696f2f736f6e6172636c6f75642d6769746875622d7374617469632d7265736f75726365732f76322f636f6d6d6f6e2f6661696c65642d313670782e706e67 C Reliability Rating on New Code https://sonarcloud.io/dashboard?id=opendcs_rest_api&pullRequest=186 (required ≥ A)

See analysis details on SonarCloud https://sonarcloud.io/dashboard?id=opendcs_rest_api&pullRequest=186

https://camo.githubusercontent.com/0d8093b061da31b2dde5907c87c133451a9fed9ab208145ec3750960f13f438a/68747470733a2f2f736f6e6172736f757263652e6769746875622e696f2f736f6e6172636c6f75642d6769746875622d7374617469632d7265736f75726365732f76322f636f6d6d6f6e2f6c696768745f62756c622d313670782e706e67 Catch issues before they fail your Quality Gate with our IDE extension https://camo.githubusercontent.com/cf28d9441c2be84b83964b59542f8523d293ea1d96a9d3cea77e7962537f0175/68747470733a2f2f736f6e6172736f757263652e6769746875622e696f2f736f6e6172636c6f75642d6769746875622d7374617469632d7265736f75726365732f76322f636f6d6d6f6e2f736f6e61726c696e742d313670782e706e67 SonarLint https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request

— Reply to this email directly, view it on GitHub https://github.com/opendcs/rest_api/pull/186#issuecomment-2350694832, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB44KCAUSJO4NPKW5DCS53LZWNXUXAVCNFSM6AAAAABN7LV2FGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJQGY4TIOBTGI . You are receiving this because you commented.Message ID: @.***>

[adamkorynta]

Yeah, I was more referring to trying to keep the DAO's operating with DTO adjacent data types, and keep Principal/sessioning up at a separate layer.

[MikeNeilson]

Yeah, I was more referring to trying to keep the DAO's operating with DTO adjacent data types, and keep Principal/sessioning up at a separate layer.

Okay, that's what I thought after reading it more, though I would still posit that returning a full javax.security.Principal is reasonable. But I'm not set one it enough to do anything but provide the suggestion. The interface looks nice and generic enough to me know.

[sonarcloud[bot]]

Quality Gate passed

Issues
45 New issues
0 Accepted issues

Measures
0 Security Hotspots
44.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[MikeNeilson]

Oh right that particular colulm is populated by the user managmemt system and not used in CWMS.

On Mon, Sep 30, 2024, 1:58 PM Adam Korynta @.***> wrote:

@.**** commented on this pull request.

In opendcs-rest-api/src/main/java/org/opendcs/odcsapi/sec/cwms/CwmsAuthorizationDAO.java https://github.com/opendcs/rest_api/pull/186#discussion_r1781726720:

@@ -44,11 +44,18 @@ public Set getRoles(String username) throws DbException { Set roles = EnumSet.noneOf(OpenDcsApiRoles.class); roles.add(OpenDcsApiRoles.ODCS_API_GUEST);

• String q = "SELECT user_group_id" +
• " FROM av_sec_users" +
• " WHERE db_office_code = cwms_util.get_db_office_code(?)" +
• " AND username = ?" +
• " AND is_member = 'T'";
• String q = "select user_group_id " +
• "from cwms_20.av_sec_users " +
• "where db_office_code = cwms_20.cwms_util.get_db_office_code(:input_db_office_code) " +
• " and upper(username) = case " +
• " when instr(:username_str, '.', -1) > 0 then " +
• " (select userid " +
• " from cwms_20.at_sec_cwms_users " +
• " where edipi = substr(:username_str, instr(:username_str, '.', -1) + 1)) " +

av_sec_users is the one with the office code, at_sec_cwms_users has an 'office' column looks to be used for something else (it's null in our database exports and 16 bytes).

— Reply to this email directly, view it on GitHub https://github.com/opendcs/rest_api/pull/186#discussion_r1781726720, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB44KCF36CIJM3TLSLQGE4DZZG3NXAVCNFSM6AAAAABN7LV2FGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMZYGU2TAOJZGE . You are receiving this because you commented.Message ID: @.***>