edgedb / edgedb-ui

The home of EdgeDB UI and all related shared UI components
Apache License 2.0
49 stars 7 forks source link

Can't display the AuthConfig table with a secret property? #282

Closed MiroslavPetrik closed 8 months ago

MiroslavPetrik commented 8 months ago

Seems like the new ext::auth added support for secret fields, which do not work in the UI?

Screenshot 2023-11-02 220417

A secret modifier is used in the extension's DDL.

It got me wondering if it's usable in classical SDL, but no luck:

type User {
      required password: str {
        secret := true; 
        readonly := true;
      }
}

The readonly property is valid but the secret throws error:

error: 'secret' is not a valid field
   ┌─ C:\fakepath\default.esdl:10:9
   │
10 │         secret := true;
   │         ^^^^^^^^^^^^^^^ error

edgedb error: cannot proceed until .esdl files are fixed
MiroslavPetrik commented 8 months ago

I speculated that I need to login into the UI as some root user, but apparently the edgedb ui command authenticates with the creadentials from the config linked via edgedb info

MiroslavPetrik commented 8 months ago

As a sidenote, i was wondering why there are the 2 configs since the auth extensions acts like a singleton. This is not only in the UI, but also in REPL. When I attempt to update the 'table' I get back error, that the view cannot be updated. It got me wondering whether one of the objects is table and the other some view of it, like a clone/mirror. But no, neither of those are updatable.

image

The values in the auth config can be updated via the configure command like configure current database set ext::auth::AuthConfig::allowed_redirect_urls := {"http://localhost:4000/", "http://localhost:3000/"}

jaclarke commented 8 months ago

i was wondering why there are the 2 configs since the auth extensions acts like a singleton

I'm not entirely sure, but I think it's some quirk of how the config system works. Other configs can have different levels like 'instance' and 'database', that get layered on each other to produce a 'final' config, which I think shows up in introspection as another config object (eg. try running select cfg::AbstractConfig {*}. I get three objects returned with types of cfg::Config, cfg::DatabaseConfig and cfg::InstanceConfig). So for auth config, where you can only define it at the database level, I think the two objects represent database and 'final' config (which end up being the same).

When I attempt to update the 'table' I get back error, that the view cannot be updated. It got me wondering whether one of the objects is table and the other some view of it, like a clone/mirror. But no, neither of those are updatable.

As far as I know all config is readonly in normal edgeql queries, and can only be updated with the configure ... commands. I think because config is special in that it's not just data in the database, but represents state that needs special handling to sync with the EdgeDB server and postgres backend (eg. listen addresses, query working memory, etc.)

msullivan commented 8 months ago

i was wondering why there are the 2 configs since the auth extensions acts like a singleton

I'm not entirely sure, but I think it's some quirk of how the config system works. Other configs can have different levels like 'instance' and 'database', that get layered on each other to produce a 'final' config, which I think shows up in introspection as another config object (eg. try running select cfg::AbstractConfig {*}. I get three objects returned with types of cfg::Config, cfg::DatabaseConfig and cfg::InstanceConfig). So for auth config, where you can only define it at the database level, I think the two objects represent database and 'final' config (which end up being the same).

Yeah, this is all correct. One AuthConfig object represents just the database level config, and one represents the final config (which is is database+session). Currently extension configs can't be configured at the session level, and so this is redundant, but that will change soon, and we didn't want people to write code that expected only one object for extension configuration and have it break. (An example of something that we really want to have be session configurable for extensions is the probes config for pgvector. Possibly it won't actually make sense for any of auth to be session configurable, so maybe we should make it so there is only one AuthConfig object, though that would require some adjusting of the schema because ExtensionConfig has a single link to the owning AbstractConfig.)

The best way to fetch an extension config object is something like

select cfg::Config.extensions[is ext::auth::AuthConfig]
msullivan commented 8 months ago

We're adding the secret field to introspection, which will allow this to be fixed in the right way, but I think we should hack around it since I don't think that fix can be out until 5.0