IdentityPython / oidc-op

An implementation of an OIDC Provider (OP)
Apache License 2.0
64 stars 26 forks source link

Add pkce essential per client #115

Closed nsklikas closed 3 years ago

nsklikas commented 3 years ago

This allows us to have pkce(essential) activated for only some clients

peppelinux commented 3 years ago

It looks good, I Just want to ask to you to put a brief description of this feature in the docs

Probably this Is good Moment to review the general policy of oidc-op with pkce, before merging this

nsklikas commented 3 years ago

What is the right place to document this?

peppelinux commented 3 years ago

https://github.com/IdentityPython/oidc-op/blob/master/docs/source/contents/conf.rst#add_on

What do you think to have a separate section for add_ons?

nsklikas commented 3 years ago

Sure

nsklikas commented 3 years ago

Do you think something like this https://github.com/nsklikas/oidc-op/blob/pkce-per-client/docs/source/contents/conf.rst#pkce ok?

peppelinux commented 3 years ago

Yes, that's great! I have to be honest, I'd wait some days before having a new roadmap for the next release, including all these precious PRs.

For now I'd like to have all the PR with some good contributions in the docs as well. You made a great job, after these days of vacation we'll have a new release

rohe commented 3 years ago

I guess the

essential = essential = endpoint_context.args["pkce"].get(

line is a typo ? Shouldn't it be just

essential = endpoint_context.args["pkce"].get(

nsklikas commented 3 years ago

Good catch.

peppelinux commented 3 years ago

@rohe do we think that this could be merged as It Is or do we have to put something more in?

rohe commented 3 years ago

I think we can go ahead with this as it is.