Implementation of Auth Token

[kawcl]

Is your feature request related to a problem? Please describe. We want to add auth tokens to our requests to Upgrade. However if we turn on the requirement for auth token validation in the Upgrade production environment it requires that all existing applications that use that environment would also need to add a token to their requests. If they don't, it'd break the implementation.

Describe the solution you'd like Do we need to start the process of making sure every application that uses production Upgrade is outfitted with an auth token or do we start requiring an auth token on an app-to-app basis?

[kawcl]

We can bring in our security team into the conversation since they are the ones who requested this update.

[jerith666]

Token checking is implemented in ClientLibMiddleWare.use().

It looks like it expects a JSON web token encrypted with the secret defined at config.clientApi.secret. That in turn is defined in config.json.

I don't see any support in the Java client lib for creating a valid JWT given the correct secret; is there any in the TS client lib?

[danoswaltCL]

Not in the library itself, the client libs just accept an optional auth token to pass along in headers if supplied by the client, but I suppose that having the library do this on its own if supplied the correct secret/api-key would be a good way to do it if it's secure enough for the client to sign it itself?

That's the first I've seen this config.json file, that context is strange looking. Those aren't the values we have in the actual config in prod for the CLIENT_API_KEY and CLIENT_API_SECRET, but it does match the values in that commented-out section of ClientLibMiddleware, so someone must have been testing this stuff out at some point long ago.

@amurphy-cl @zackcl @bcb37 @VivekFitkariwala thoughts?

[danoswaltCL]

So others know, I didn't realize this was how it was working:

AUTH_CHECK environment variable is for checking the presence of a bearer token on non-client-library requests.

setting.toCheckAuth is a different thing (!). in the client lib middleware does not get this from the env config, it gets it from the database, there's a "settings" table with a column called toCheckAuth.

Currently this is FALSE in production, whereas AUTH_CHECK is TRUE, so non-client-library requests need the auth token, but client-lib requests do not.

Does anyone have any memory of why this was set up this way? (using a settings table in db for a couple of things instead of in env var list)? @VivekFitkariwala @ppratikcr7

[jerith666]

I don't know why it was built this way, but there's a toggle in the UI that lets you flip toCheckAuth. Always seemed like a bad idea to me to expose it to any old experimenter like that, though. :)

[jerith666]

Hrm, maybe it's been removed from the UI? Either that or I no longer have permission to see it. :)

[VivekFitkariwala]

@danoswaltCL config.json is no longer used and it can be deleted. Those data were moved to environment variables. We have tested the auth code with the JWT shared secret.

[VivekFitkariwala]

@danoswaltCL AUTH_CHECK refers to the process of verifying if a user is signed in using their Google account. This is used to allow HTTP requests to be made without requiring the user to provide their Google authentication-related details. We can modify it to have a default value "true"; however, during development, users can set it to "false" if needed.

toCheckAuth parameter determines whether the client-side request should verify the JWT token before processing.

[amurphy-cl]

@jerith666 are you looking for the auth toggle in prod? I just changed your permissions so you should be able to see it now, if you want (to your other point, updating permissions structure is on the 2024 roadmap).

[amurphy-cl]

@VivekFitkariwala let's definitely delete that config file since it's no longer being used

[jerith666]

Yeah, I just wanted to grab a screenshot of the UI in question for the record:

(You can downgrade my permissions again now if you want. ;-) )

[amurphy-cl]

@jerith666 your permissions are correct now 😊 They'd been set lower previously because of a default action that is on the agenda to fix!

[danoswaltCL]

Reviewing all of this, I think we definitely need to disable users from being allowed to toggle client-endpoint auth in the UI. that is a disaster waiting to happen in prod.

I think we would just want this also to be an env var like AUTH_CHECK for Google Credential UI requests, I don't think there's a reason to need to toggle it on the fly without a config reboot, but then again there was probably some reason we did it that way in the beginning...?

@amurphy-cl @VivekFitkariwala

EDIT: Actually, thankfully perhaps, this auth toggle is broken. The /settings endpoint validation is catching a bad request from the UI, it expects 2 properties but it only sends one. I can see it break on bad request in dev. I'm not going to try it in prod, but the validation change to true is in there for v5 as well. It used to be off, so this toggle may have been functional in v4 and lower.

Toggling can be done via POST /settings API directly with correct object.

[ppratikcr7]

Reviewing all of this, I think we definitely need to disable users from being allowed to toggle client-endpoint auth in the UI. that is a disaster waiting to happen in prod.

I think we would just want this also to be an env var like AUTH_CHECK for Google Credential UI requests, I don't think there's a reason to need to toggle it on the fly without a config reboot, but then again there was probably some reason we did it that way in the beginning...?

@amurphy-cl @VivekFitkariwala

EDIT: Actually, thankfully perhaps, this auth toggle is broken. The /settings endpoint validation is catching a bad request from the UI, it expects 2 properties but it only sends one. I can see it break on bad request in dev. I'm not going to try it in prod, but the validation change to true is in there for v5 as well. It used to be off, so this toggle may have been functional in v4 and lower.

Toggling can be done via POST /settings API directly with correct object.

@danoswaltCL Now we have resolved and merged the fix to the broken auth toggle. So, we need to take this ticket and decide if we need to remove this toggle option.