juanifioren / django-oidc-provider

OpenID Connect and OAuth2 provider implementation for Djangonauts.
http://django-oidc-provider.readthedocs.org
MIT License
419 stars 239 forks source link

Consistent ordering of RSAKeys is not guaranteed #315

Open tituomin opened 5 years ago

tituomin commented 5 years ago

Django doesn't guarantee a consistent ordering of models without an explicit default ordering.

https://github.com/juanifioren/django-oidc-provider/blob/f0daed07b2ac7608565b80d4c80ccf04d8c416a8/oidc_provider/lib/utils/token.py#L158

Rotating RSA keys requires a consistent ordering, because the first key in the set of keys is always used for signing. Should we add a default ordering?

See https://stackoverflow.com/a/7164126 and https://docs.djangoproject.com/en/2.1/ref/models/options/#ordering

tituomin commented 5 years ago

I can make a pull request if there are no objections.

tituomin commented 5 years ago

To be more precise: I suppose an ordering of the keys is not strictly necessary, since the clients can always fetch the keys when it encounters a new key id. But always signing with the oldest valid key might help clients with cached keys to avoid extra requests.

toti1212 commented 5 years ago

Its suppose that the order was by id? Or you think to add a new field like created_at?

tituomin commented 5 years ago

Ordering by id should be fine. The default primary key in Django should auto-increment by default. The point was just to add the 'id' to the Meta option ordering: https://docs.djangoproject.com/en/2.2/ref/models/options/#ordering