More examples with more detailed comment will help!

[duckfly-tw]

Is your feature request related to a problem? Please describe.
Big thank for your efforts on this great project!
I want to build an OP based on this project, but the OP example is too fuzzy for me to make the features I want.

Describe the solution you'd like
Add or modify OP examples:

1. login with password, so it can help me safely processing login.
2. handle multiple client_id, so it can help me know how to handle multiple clients.
3. more detail comments on the example code, so I can know where I should modify and where I can keep as it is.

Appreciate your help!

[adlerhurst]

Hi @duckfly-tw

Thanks for your kind words. I will forward them to @livio-a 😉

During the Christmas season, we take a little longer because many people are on well-deserved vacation. We do our best to complete the examples.

Cheers

[raydeng83]

This is a great project and you guys are awesome to open source it.

More examples and documentation will definitely help!

Best,
Le

[adlerhurst]

Hi @raydeng83 thank you very much. @livio-a is doing a great job. Feedback like yours is why we are doing it. It makes working a pleasure 😊
If you have any suggestions please share them with us.

[muhlemmer]

Hi, are you open for pull request? I've just started working with your project and I find that the Godoc can be improved in some points. I find myself flipping through go doc, examples and source to find out how to implement the Storage interfaces. Since I',m already doing the research effort, it should take much to supplement the documentation in the process.

For example,

oidc/pkg/op/storage.go

Lines 11 to 13 in 94871af

	type AuthStorage interface {
	CreateAuthRequest(context.Context, *oidc.AuthRequest, string) (AuthRequest, error)

1. Doesn't tell me the requirements of AuthStorage is it a temporary storage to maintain state in the authentication workflow, or does it need persistence?
2. What does the third argument of type string do in CreateAuthRequest? (same for most of the method)

When digging deeper into the example code, those questions are answered:

1:

oidc/example/server/internal/storage.go

Lines 159 to 163 in 94871af

	//DeleteAuthRequest implements the op.Storage interface
	//it will be called after creating the token response (id and access tokens) for a valid
	//- authentication request (in an implicit flow)
	//- token request (in an authorization code flow)
	func (s *storage) DeleteAuthRequest(ctx context.Context, id string) error {

2:

oidc/example/server/internal/storage.go

Lines 113 to 115 in 94871af

	//CreateAuthRequest implements the op.Storage interface
	//it will be called after parsing and validation of the authentication request
	func (s *storage) CreateAuthRequest(ctx context.Context, authReq *oidc.AuthRequest, userID string) (op.AuthRequest, error) {

Therefore I would like to propose:

• Add more explicit documentation to the storage related interface types
• Use named arguments in the interface types.

For example, I would do something like:

// AuthStorage maintains states of the authentication processes and active tokens.
// Implementations may purge expired tokens or outdated authentication requests.
type AuthStorage interface { 
    // CreateAuthRequest stores a new authentication request in the database for the passed userID.
    // A unique request ID (primary key) must be assigned by the implementation, for later retrieval.
    CreateAuthRequest(ctx context.Context, r *oidc.AuthRequest, userID string) (AuthRequest, error)

    // AuthRequestByID retrieves a authentication request by its unique ID.
    AuthRequestByID(ctx context.Context, reqID string) (AuthRequest, error)

    // DeleteAuthRequest will be called after creating the token response (id and access tokens) for a valid:
    //- authentication request (in an implicit flow) 
    //- token request (in an authorization code flow) 
    DeleteAuthRequest(ctx context.Context, reqID string) error

Let me know if you're open for PRs and I'll get started ;).

[Adirelle]

Hi there,

I am in the same situation as muhlemmer. I am trying to implement an OP for development and testing purpose, but even though there is an example, the interfaces to implement lack some documentations and comments. Moreover, I have the feeling they are a bit bloated, e.g. there are too many methods in each Storage and some interfaces seem to have several responsibilities. This makes the implementation quite difficult.

[livio-a]

Hey @muhlemmer and @Adirelle

We're certainly open for PRs 😃 and would appreciate any help. And there's certainly lots of improvement possible on documentation. 😉

We started this project as we needed an OP for out own product (https://github.com/zitadel/zitadel) and wanted to separate the functionality into a package. We tried to abstract it as good as possible, but I'm sure there's room for improvement, too. If there are any suggestions on this, we can of course discuss this as well.

[Adirelle]

I think that what @muhlemmer proposes would be a good start to improve the documentation.

As for the interfaces, I think what is missing is some documentation about the entity-relation model used by the OP, and how entities are used. This could allow to write services and repository interfaces, which are easier to reason about and to implement. They could come as a separate package, with adapters to keep the compatibility with existing interfaces.

[hifabienne]

Will close this issue for now, if we see new stuff popping up we will create dedicated issues for it.