dexidp / dex

OpenID Connect (OIDC) identity and OAuth 2.0 provider with pluggable connectors
https://dexidp.io
Apache License 2.0
9.42k stars 1.69k forks source link

Better specification of password info in --no-db mode #340

Closed ericchiang closed 8 years ago

ericchiang commented 8 years ago

It's really weird that user passwords are specified in the connectors.json. It's also only valid for --no-db mode and will cause panics with Postgres (#286).

For instance, our example connector JSON file looks like this:

[
    {
        "type": "local",
        "id": "local",
        "passwordInfos": [
            {
                "userId":"elroy-id",
                "passwordPlaintext": "bones"
            },
            {
                "userId":"penny",
                "passwordPlaintext": "kibble"
            }
        ]
    }
]

Move these out of the connector and put them in the users.json file.

[
    {
        "user":{
            "id": "elroy-id",
            "email": "elroy77@example.com",
            "displayName": "Elroy Jonez",
            "passwordPlaintext": "bones"
        },
        "remoteIdentities": [
            {
                "connectorId": "local",
                "id": "elroy-id"
            }
        ]
    }
]

Since this file is only parsed for --no-db mode, it seems reasonable to put that data there.

bobbyrullo commented 8 years ago

Is this worth the effort of creating special parsing code just for this situation, since we wouldn't want to change the actual data model?

Also, it only seems really weird if you don't understand the data model - once you do it makes sense. I would worry that it would give people a misleading impression of the schema to change it.

Also, it's only for --no-db mode.

ericchiang commented 8 years ago

Is this worth the effort of creating special parsing code just for this situation, since we wouldn't want to change the actual data model?

I believe the current parsing code is only used in --no-db mode.

Also, it only seems really weird if you don't understand the data model - once you do it makes sense. I would worry that it would give people a misleading impression of the schema to change it.

This is more for getting the password info out of the local connector's metadata. We don't store password infos in the local connectors for anything but --no-db mode, so this isn't a good representation of the data model anyway.

If we remove the password infos out of the local connector's metadata we can stop trying to load passwordInfos as part of loading the connectors. Which would fix #286

bobbyrullo commented 8 years ago

It would not fix #286 - even if password infos were not in there we still should not be loading connectors from a file except in no db mode.

ericchiang commented 8 years ago

Sorry. What I meant was that if we didn't load the password infos from the connector, #290 would be a valid fix for #286