Technical Question: should we use a JWT for AUTH_API_KEY?

[nelsonic]

At present an AUTH_API_KEY has the format:

88SwQDWYGmiDACVRaBGqQghzLd5jfX7YynefCJ8M/88SwQH8CEDWzWSq7PyWe7ADpjYCpmczr5zyTK4d/authdemo.fly.dev

Note: this is not a valid key. but the format is correct.

The reason I wet with this and still like it is because it's human-readable
and immediately obvious which environment (auth instance) it's for; in this case: authdemo.fly.dev
So if I see this key in my .env file or environment variable, I know exactly what it's for. 👌
I think this has a pretty major advantage in terms of Developer Experience ... 💭

But equally it has a disadvantage: no embedded info like expiry.
We outlined the benefits of JWTs in our mega-popular doc: https://github.com/dwyl/learn-json-web-tokens
And we are using them in auth for session tokens:

auth/lib/auth_web/controllers/auth_controller.ex

Line 200 in 83c286a

|> AuthPlug.create_jwt_session(session_data(person, conn.assigns.sid))

So my question is: should the next version of auth (and auth_plug) use a JWT as the AUTH_API_KEY?
Will having a JWT for the AUTH_API_KEY and a different JWT for session token be confusing to devs? 🤷‍♂️
Are we missing something?

Please let me know your thoughts ... 💭 🙏

[LuchoTurtle]

I don't think it makes much sense to have info like expiry in the AUTH_API_KEY. In my opinion, the key should be a unique string that the application uses to guarantee that the application using it is genuine in the "auth ecosystem".

However, the JWT session token that is returned to the browser and to the user should certainly have such kind of metadata. After all, JWT is just a way of encoding claims between two parties into a string. This session token should certainly be JWT but I fail to see the usefulness of having metadata info in a key that is used as env variable - it should be present in the session token JWT.

A similar thing happens when working with OpenID Connect.

If I have a client, a server and an identity provider like Keycloak, for example, the server uses in deployment a key as env variable that the identity provider issues, so it knows the server is legitimate. This key doesn't need to be a JWT.

However, if I were to use an OIDC (OpenID Connect) flow for users to authenticate in the server...

the interactions between the client and the server are made through JWT tokens. But the web server securely communicates with the identity provider to check if the issued ID tokens and Access Tokens (part of the OIDC authentication flow) are legitimate. And the only reason the identity provider is the provider for the server, is because it gave the server "a key" so it knows it's a legitimate application to be communicated with. This happens during deployment and does not necessarily mean the key is a JWT.

tldr:

JWT = for communication between client and web servers
AUTH_API_KEY = doesn't really need to be JWT, I don't see how it is useful. It's just a way for the auth application to know the server is legitimate.

[nelsonic]

@LuchoTurtle thanks for your insightful reply. 👌
Yeah, I was trying to rack my brain to figure out how using a JWT as the AUTH_API_KEY would be useful.
The only thing I can think of is validity checking on the side of the "consuming" app ...
A couple of days ago we saw the client_id XYZ is not valid error: dwyl/phoenix-chat-example#150

To the person using auth_plug this is not a very helpful error; a potential time-waster... ⏳ 💸 🔥
If the AUTH_API_KEY was a JWT, we could easily add a AuthPlug.verify_jwt/1 function
and thus avoid this class of errors completely.
Obviously we can (significantly) improve our error handling/reporting in auth ...
but if there was a way of checking the AUTH_API_KEY with a function,
it would eliminate the need for handling this type of error
because the engineer would immediately know if their AUTH_API_KEY is valid
and could even test it independently on https://jwt.io/ ✅

Hmm ... I'm still not "sold" on this in terms of cost-benefit. 🤔

[SimonLab]

I'm adding a bit more context as it took me a bit of time to understand the question:

To be able to use the auth application you first have to create an application, see https://github.com/dwyl/auth_plug#2-get-your-auth_api_key

The create action in the app_controlller is called:

auth/lib/auth_web/controllers/app_controller.ex

Lines 14 to 30 in 83c286a

	def create(conn, %{"app" => app_params}) do
	attrs =
	Map.merge(app_params, %{
	"person_id" => conn.assigns.person.id,
	"status" => 3
	})
	case App.create_app(attrs) do
	{:ok, app} ->
	conn
	|> put_flash(:info, "App created successfully.")
	|> redirect(to: Routes.app_path(conn, :show, app))
	{:error, %Ecto.Changeset{} = changeset} ->
	render(conn, "new.html", changeset: changeset)
	end
	end

Which calls App.create_app/1:

auth/lib/auth/app.ex

Lines 96 to 108 in 83c286a

	def create_app(attrs \\ %{}) do
	case %App{} |> App.changeset(attrs) |> Repo.insert() do
	{:ok, app} ->
	# Create API Key for App https://github.com/dwyl/auth/issues/97
	Auth.Apikey.create_apikey(app)
	# return the App with the API Key preloaded:
	{:ok, get_app!(app.id)}
	{:error, err} ->
	{:error, err}
	end
	end

Which then calls Auth.Apikey.create_apikey(app) :

auth/lib/auth/apikey.ex

Lines 36 to 38 in 83c286a

	def create_api_key(id) do
	encrypt_encode(id) <> "/" <> encrypt_encode(id)
	end

Finally encrypt_encode(id) is defined as:

auth/lib/auth/apikey.ex

Lines 27 to 29 in 83c286a

	def encrypt_encode(plaintext) do
	plaintext |> Fields.AES.encrypt() |> Base58.encode()
	end

Just spotted this while I was doing this recap and I'm not sure if this is intentional here or a bug:
We are passing the full app struct to Auth.Apikey.create_apikey(app).
From the definition of the other functions def create_api_key(id) do and def encrypt_encode(plaintext) do it seems that we should pass the app.id instead or its string representation (plaintext?)
This works because the encrypt function in Fields is defined here https://github.com/dwyl/fields/blob/main/lib/aes.ex#L29-L30 with

  @spec encrypt(any) :: String.t()
  def encrypt(plaintext) do

the argument can by of any types, but I still think app.id might be what was intended? Let me know if this makes sense.
Looked at the wrong function, this is the correct one:

auth/lib/auth/apikey.ex

Lines 65 to 77 in 83c286a

	def create_apikey(app) do
	attrs = %{
	"client_secret" => encrypt_encode(app.id),
	"client_id" => encrypt_encode(app.id),
	"person_id" => app.person_id,
	"status" => 3,
	"app" => app
	}
	%Apikey{}
	|> Apikey.changeset(attrs)
	|> Repo.insert()
	end

But to come back to the auth api key we see that it is created using Fields and the format is client_id/secret_id`

Now when a user authenticate the auth app is using jwt to encode the person's info

auth/lib/auth_web/controllers/auth_controller.ex

Line 200 in 83c286a

|> AuthPlug.create_jwt_session(session_data(person, conn.assigns.sid))

Now to come back to the error describe above #268 (comment)

client_id: .... is not valid

It is defined here:

auth/lib/auth_web/controllers/auth_controller.ex

Lines 17 to 30 in 83c286a

	def index(%{query_params: %{"auth_client_id" => client_id}} = conn, params) do
	valid_client_id = client_id && client_id_valid?(client_id, conn)
	log_auth(conn, params, client_id, valid_client_id)
	if valid_client_id do
	render_login_buttons(conn, params)
	else
	error_message = "client_id: #{client_id} is not valid"
	conn
	|> put_flash(:error, error_message)
	|> unauthorized(error_message)
	end
	end

By calling client_id_valid?:

auth/lib/auth_web/controllers/auth_controller.ex

Lines 81 to 95 in 83c286a

	def client_id_valid?(client_id, conn) do
	# attempt to decrypt the client_id
	case Auth.Apikey.decode_decrypt(client_id) do
	# if auth_client_id in the URL but we cannot decrypt it, reject early!
	# see: https://github.com/dwyl/auth/issues/129
	{:error, _} ->
	msg = "client_id_valid?:109 Unable to decrypt auth_client_id:#{client_id}"
	Auth.Log.error(conn, %{msg: msg})
	false
	# able to decrypt the client_id to an app_id check if still valid:
	{:ok, app_id} ->
	client_id_is_current?(app_id, client_id)
	end
	end

I think the error message could be better, something like "the auth_api_key used with auth_plug is not linked to any applications on the auth app"?

Note also that the decode_decrypt function is defined as:

auth/lib/auth/apikey.ex

Lines 44 to 52 in 83c286a

	def decode_decrypt(key) do
	try do
	decrypted_key = key |> Base58.decode() |> Fields.AES.decrypt() |> String.to_integer()
	{:ok, decrypted_key}
	rescue
	_ ->
	{:error, "invalid key"}
	end
	end

Is there also a bug with decrypted_key = key |> Base58.decode() |> Fields.AES.decrypt() |> String.to_integer() ? I don't understand why String.to_integer() is used here, are we sure that the string returned by Field.AES.decrypt` can be converted to an integer, especially if when we first encoded, the key might have represented the app struct and not just the id (see above)

Looked again and the app.id is correctly encoded, I looked at create_api_key instead of create_apikey. We might want to rename some of the functions to avoid this mistake

But I don't think it makes sense to use jwt for the auth api key. I can't see a use case where we want to define an expired time to be then checked on the application with auth_plug. When an application is created on auth I don't think we want it to be inaccessible after a period of time?

Let me know if all this makes sense.

[LuchoTurtle]

Thanks for the comprehensive explanation @SimonLab , makes much more sense now. I had some run-ins with this error but now knowing how it works helps a lot!
I'd open an issue but since auth is in the process of being rebuilt, I wager it's not useful. As @nelsonic said, "it's like re-arranging deck chairs on the Titanic 🚢"

[nelsonic]

Thanks for both your feedback on this. Very good that we're all thinking on similar lines. 💭
Totally agree that this is confusing and potentially error-prone. 😕
Very much going to try and simplify the steps for creating an AUTH_API_KEY. 🔑
So that people using the API or trying to run/build the App on their localhost have only 1 step: copy.

[nelsonic]

@SimonLab thanks for stepping through the existing code to give context.
I actually didn't want to get bogged down in the existing code
because I've already agreed it's over-complicated.
All I wanted to discuss was if AUTH_API_KEY should be a JWT or not.
However ... it has prompted me to open a follow-up question: Do we even need an AUTH_API_KEY: #277

[LuchoTurtle]

Since some of this discussion was migrated to #277, should this issue be closed? The usage of the very existence of an AUTH_API_KEY was debated in #277.

Although it's confirmed in #277 (comment) that we'll have an AUTH_URL/AUTH_API_KEY, it stated that it will have a auth.domain.com/:person_id/:public_key format, not JWT. 💭

[nelsonic]

Indeed. Not using JWT in AUTH_API_KEY. ✅
Thanks.