Feature/1092 1098 basic user management #1115

MikeNeilson · 2025-05-29T22:06:09Z

I think there's still a bit more to do, but ready for discussion. Plus the tests just started failing on my laptop after adding one more (like fail to even start correctly) so checking on github.

MikeNeilson · 2025-06-02T15:42:40Z

NOTE: these are a stop gap until further work is done on authentication and so I'm not super worried about getting all the timing information in and such.

I did on one, but only a handful of people are going to be using this, and even then not super often.

krowvin · 2025-06-06T20:55:44Z

cwms-data-api/src/main/java/cwms/cda/ApiServlet.java

+        crud("/users/{user-name}", new UsersController(metrics), adminRoles);
+        get("/roles", new GetRolesController(metrics), adminRoles);
+        get("/user/profile", new UserProfileController(metrics), userRoles);
+        post("/user/{user-name}/roles/{office-id}", new AddRoleController(metrics), adminRoles);


Does this suggest that a user could have a space in multiple offices?

As opposed to
/user/{office-id}/roles/{user-name}?

I would just think that office would be the base and a user would be requested from that office

This probably doesn't really matter since either way the values get passed in. It's just, to me an office seems like it would come first.

For most of the other data you would be correct, but in this case I don't think so.
The user is a the root of the infrastructure, and permissions to an office data are secondary.
E.g. the user "belongs" to the system as a whole.

Versus say a Time Series which "belongs" to a specific office.

krowvin · 2025-06-06T20:56:37Z

cwms-data-api/src/main/java/cwms/cda/api/auth/users/UserProfileController.java

+        security = {
+            @OpenApiSecurity(name = "gets overridden allows lock icon.")
+        },
+        description = "View users's own information",


user's* I believe

Unless you meant multiple users then disregard!

actually it should be users', but technically users's is valid. in that sentence it's both plural and "owned."

I think. I'm certainly not going to claim I got excellent marks in English classes, but I often remember that esoteric stuff fairly well.

krowvin · 2025-06-06T20:58:55Z

cwms-data-api/src/main/java/cwms/cda/api/auth/users/UsersController.java

+            @OpenApiParam(allowEmptyValue = true, name = OFFICE, type = String.class,
+                    description = "Show only users with active privileges in a given office" ),
+            @OpenApiParam(name = PAGE,
+                    description = "This end point can return a lot of data, this "


This end point can return a lot of data

I don't know if this needs to be said, but I get the spirit of it

"Lots of data, don't forget about the page option"

krowvin · 2025-06-06T21:00:05Z

cwms-data-api/src/main/java/cwms/cda/api/auth/users/UsersController.java

+
+    @OpenApi(
+        pathParams = {
+            @OpenApiParam(name = "user-name", required = true,


username or user-name - to hyphen or not to hyphen

That is the question.

krowvin · 2025-06-06T21:02:31Z

cwms-data-api/src/main/java/cwms/cda/api/auth/users/roles/AddRoleController.java

+    @OpenApi(
+        pathParams = {
+            @OpenApiParam(name = "office-id", required = true,
+            description = "Office for these roles"),


I still think this should just be generic

I.e. in one place create a definition with a link to the offices endpoint

String OFFICE_DESCRIPTION = "Office Identifier 3/4 letter as returned by the office endpoint: " + OFFICE_ENDPOINT

Throwing this idea out there again

Fair point, and it might already exist and I should use it.

krowvin · 2025-06-06T21:04:44Z

cwms-data-api/src/main/java/cwms/cda/data/dao/UserDao.java

+
+    private static final String GET_USER =
+        "select ut.userid as username,ut.email, ut.principle_name,groups.db_office_id as office,groups.user_group_id as role " +
+        " from cwms_20.at_sec_cwms_users ut " +


I know we are not making more plsql packages/etc (raw SQL)

But how much longer will cwms_20 really be around (usage of cwms_20)

Gotta do something to move forward I suppose

"CWMS_20" forever, or at least until we've moved to Postgres (but to be fair maybe even then).

But as for the creation of PLSQL package, only when it improves/simplifies usage or is required for specific permissions issues.
In this case there's no benefit to doing so.

krowvin · 2025-06-06T21:08:01Z

cwms-data-api/src/main/java/cwms/cda/data/dao/UserDao.java

+        return dsl.connectionResult(c -> {
+            AuthDao.setSessionForAuthCheck(c);
+            final ArrayList<String> roles = new ArrayList<>();
+            try (PreparedStatement getUser = c.prepareStatement("select distinct user_group_id from cwms_20.at_sec_user_groups order by user_group_id asc");


Is there not a jooq equivalent for this?

i.e.

Result<Record1<String>> result = DSL.using(configuration) .selectDistinct(AT_SEC_USER_GROUPS.USER_GROUP_ID) .from(AT_SEC_USER_GROUPS) .orderBy(AT_SEC_USER_GROUPS.USER_GROUP_ID.asc()) .fetch();

Or does it fall short because you need a PreparedStatement?

There's no code gen for these tables, and it's too much work to make that happen just for this change.

krowvin

Just tossing my thoughts out there!

rma-rripken

Are there unit tests for User and Users? JSON test resources make it really obvious what the input/outputs look like.

cwms-data-api/src/main/java/cwms/cda/api/auth/users/UsersController.java

cwms-data-api/src/main/java/cwms/cda/data/dao/UserDao.java

MikeNeilson · 2025-06-11T19:48:15Z

Are there unit tests for User and Users? JSON test resources make it really obvious what the input/outputs look like.

I'll add them, may help with sorting out the pagination cursor anyways.

MikeNeilson · 2025-06-17T21:02:49Z

@rma-rripken FYI, I have hit a snag with some insanity in the database tables. I'm going to take a break before I throw the database itself off a cliff.... I don't know how I would do it, I just know I would try to make it happen.

…the cursor value.

MikeNeilson · 2025-06-20T19:29:53Z

@rma-rripken there's likely some slop you'll have to point out to be, but it all actually behaves now. Had some definite issues in the paging design itself.

rma-rripken

Looks good to me. I'd still like to see unit test for User and Users serialization with json test resource. I find those helpful to see what the actual json looks like.

MikeNeilson added 9 commits May 21, 2025 17:59

Initial layout of user/role management controllers.

34ba0f5

Initial structure of the endpoints set.

8cb8a91

Initial structure of the endpoints set.

db6df3e

Move cac auth flag to own variable.

6e544de

Roles can be added and removed.

4971c5b

Able to retrieve roles.

42cd77a

WIP on get all users.

76f98d8

Prelimiary Operations complete.

a47888e

Add test to verify unauthorized access to user list fails.

9c42037

MikeNeilson requested review from jbkolze and krowvin May 29, 2025 22:06

MikeNeilson requested a review from rma-psmorris June 2, 2025 15:42

MikeNeilson added the approved-W912HQ25F0116 Only valid if set by MikeNeilson, DanielO, CharlesG label Jun 2, 2025

This was linked to issues Jun 2, 2025

Create auth/user end point for current user information. #1092

Open

Create basic endpoints for account management #1098

Open

MikeNeilson added 3 commits June 4, 2025 13:59

Rename controller for user data.

c2984ca

Additional logging for ATO requirements.

8871714

Merge branch 'develop' into feature/1092-1098-basic-user-management

1535305

krowvin reviewed Jun 6, 2025

View reviewed changes

Merge branch 'develop' into feature/1092-1098-basic-user-management

b551974

rma-rripken requested changes Jun 11, 2025

View reviewed changes

cwms-data-api/src/main/java/cwms/cda/api/auth/users/UsersController.java Outdated Show resolved Hide resolved

cwms-data-api/src/main/java/cwms/cda/data/dao/UserDao.java Outdated Show resolved Hide resolved

cwms-data-api/src/main/java/cwms/cda/data/dao/UserDao.java Show resolved Hide resolved

Merge branch 'develop' into feature/1092-1098-basic-user-management

931899c

MikeNeilson added 3 commits June 16, 2025 08:22

Merge branch 'develop' into feature/1092-1098-basic-user-management

12f0d9d

initial PR review updates.

6c70118

Users get all cursor behaving correctly, still no roles returned.

507b115

MikeNeilson added 2 commits June 17, 2025 22:04

Add original 'limit' office to cursor, rebuild the where clause with …

68cd749

…the cursor value.

Handle null limiit office in cursor correctly.

313eb39

MikeNeilson requested a review from rma-rripken June 20, 2025 19:29

rma-rripken approved these changes Jun 20, 2025

View reviewed changes

Feature/1092 1098 basic user management #1115

Are you sure you want to change the base?

Feature/1092 1098 basic user management #1115

Uh oh!

Conversation

MikeNeilson commented May 29, 2025

Uh oh!

MikeNeilson commented Jun 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krowvin Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krowvin Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krowvin left a comment

Choose a reason for hiding this comment

Uh oh!

rma-rripken left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MikeNeilson commented Jun 11, 2025

Uh oh!

MikeNeilson commented Jun 17, 2025

Uh oh!

MikeNeilson commented Jun 20, 2025

Uh oh!

rma-rripken left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

krowvin Jun 6, 2025 •

edited

Loading

krowvin Jun 6, 2025 •

edited

Loading