Don't automatically cast all env-vars to numbers

[cupcakearmy]

Preflight Checklist

• [X] I have completed all Troubleshooting Steps.
• [X] I'm on the latest version of Directus.
• [X] There's no other issue that already describes my problem.

Describe the Bug

I'm trying to get openid to work but no luck.

version: '3.8'

services:
  db:
    image: postgres:14-alpine
    restart: unless-stopped
    env_file: .env
    volumes:
      - ./data/db:/var/lib/postgresql/data

  cache:
    image: redis:6-alpine
    restart: unless-stopped

  directus:
    image: directus/directus:9
    restart: unless-stopped
    env_file: .env
    volumes:
      - ./data/uploads:/directus/uploads
    depends_on:
      - cache
      - db
    ports:
      - 80:8055

POSTGRES_USER=test
POSTGRES_DB=test
POSTGRES_PASSWORD=***

KEY=***
SECRET=***
ACCESS_TOKEN_TTL=1d
REFRESH_TOKEN_TTL=30d

PUBLIC_URL=http://localhost

AUTH_PROVIDERS="slack"

AUTH_SLACK_DRIVER="openid"
AUTH_SLACK_CLIENT_ID="***"
AUTH_SLACK_CLIENT_SECRET="***"
AUTH_SLACK_ISSUER_URL="https://slack.com/.well-known/openid-configuration"
AUTH_SLACK_ALLOW_PUBLIC_REGISTRATION=true

DB_CLIENT=pg
DB_HOST=db
DB_PORT=5432
DB_DATABASE=test
DB_USER=test
DB_PASSWORD=***

CACHE_ENABLED=true
CACHE_STORE=redis
CACHE_REDIS=redis://cache:6379

ADMIN_EMAIL=admin@example.org
ADMIN_PASSWORD=changeme

I've tested the OpenID flow with https://openidconnect.net/ and it works.

What is interesting, looking at the code

It should be an error before if clientId would not be defined here

However the error comes from the openid-client library. Am I missing something? Thanks & I hope it's not a duplicate, could not find anything

To Reproduce

Get the latest release v9.0.0 and try slack oauth or openid

Errors Shown

Stacktrace:

directus-directus-1  | 12:37:25 ✨ Initializing bootstrap...
directus-directus-1  | 12:37:25 ✨ Database already initialized, skipping install
directus-directus-1  | 12:37:25 ✨ Running migrations...
directus-directus-1  | 12:37:25 ✨ Done
directus-directus-1  | npm notice 
directus-directus-1  | npm notice New patch version of npm available! 8.1.0 -> 8.1.3
directus-directus-1  | npm notice Changelog: <https://github.com/npm/cli/releases/tag/v8.1.3>
directus-directus-1  | npm notice Run `npm install -g npm@8.1.3` to update!
directus-directus-1  | npm notice 
directus-directus-1  | 12:37:28 ⚠️  PostGIS isn't installed. Geometry type support will be limited.
directus-directus-1  | 12:37:29 ✨ Server started at http://localhost:8055
directus-directus-1  | /directus/node_modules/openid-client/lib/client.js:173
directus-directus-1  |       throw new TypeError('client_id is required');
directus-directus-1  |             ^
directus-directus-1  | 
directus-directus-1  | TypeError: client_id is required
directus-directus-1  |     at new BaseClient (/directus/node_modules/openid-client/lib/client.js:173:13)
directus-directus-1  |     at new Client (/directus/node_modules/openid-client/lib/client.js:1742:7)
directus-directus-1  |     at /directus/node_modules/directus/dist/auth/drivers/openid.js:34:25
directus-directus-1  |     at processTicksAndRejections (node:internal/process/task_queues:96:5)
directus-directus-1 exited with code 1

What version of Directus are you using?

v9.0.0

What version of Node.js are you using?

The one in the official docker image

What database are you using?

postgres:14-alpine

What browser are you using?

FF

What operating system are you using?

macOS

How are you deploying Directus?

Docker

[rijkvanzanten]

Odd! I just copy pasted your example (including the ***), and I'm not seeing that error on startup 🤔

Could there be a discrepancy in how those env vars are loaded into Docker? One thing I noticed is that the env vars don't use quotes, except for the AUTH ones, maybe that doesn't fly with Docker's way of loading in the .env file?

[rijkvanzanten]

(@danilopolani this is exactly the sort of config gotcha that prevents us from doing cool things like in your PR 😄 )

[cupcakearmy]

Took me a good 30 min but I got it: Turn out Slack Client IDs are a long string of digits and have a . inside. (e.g. xxxxxxxxxxxxxx.xxxxxxxxxxxxxxxx) Having a dot inside of the client id causes the break. Why?

Well what happens is that here directus thinks it's a number because of the isNan function and casts it to a number. Now since the leading number is quite big, decimal places are lost, altering the value. I guess from then the openid-client library only accepts a string and not a number

So I don't know how much numbers are required in the config file, so can't tell if this is a required feature, however makes oauth/openid not usable with slack.

[batuhanbilginn]

Yeah, it's the same for Facebook Client ID.

[rijkvanzanten]

You should be able to force cast it to a string by using AUTH_SLACK_CLIENT_ID="string:xxx.xxx.xxx"

We'll leave this open, as I agree we should probably not cast things to numbers by default, and instead rely on a hardcoded list of env vars that we expect to be numbers 👍🏻

[cupcakearmy]

Can confirm that with `"string:..." it works. Maybe a small "watch out" section in the docs would be helpfull :) Thanks for the quick replies as always ❤️

[rijkvanzanten]

Linear: ENG-287

[ched-dev]

I just ran into this issue with facebook OAuth login. They use all numbers for their CLIENT_ID which was giving me an error:

TypeError: client_id is required

I'm using Directus 10.3.0