User Experience and Feedback on Issues

[xslasd]

I am honored to be able to use the zitadel/oidc library. I have been following and learning it since early versions. Thank you to the author for their hard work!

I would like to share my user experience and hope it can be helpful for the updates of the version.

Motivation for me to continue learning and using

1. The current library has powerful functions and supports OpenID authentication, with business processes conforming to OpenID standards.
2. The code hierarchy is relatively clear, and the custom storage API can better integrate with my project.
3. This library has cases of zitadel/zitadel, which I have more confidence in using
4. The update frequency of this project. (But this is bidirectional, indicating that there is still more room for optimization in the functionality)

Some issues in use

1. Insufficient usage examples and incomplete documentation. I spend a lot of time understanding functions by reading source code.
2. In this project, the third-party library github.com/gorilla/mux was used and we hope to replace it with local net/HTTP. This would allow for better integration into existing frameworks without the need for Gorilla/Linux libraries in the current project.
3. Some parts of this project use type assertions, and I think we can reduce some assertion judgments. for example:

func needsRefreshToken(tokenRequest TokenRequest, client AccessTokenClient) bool {
	switch req := tokenRequest.(type) {
	case AuthRequest:
		return strings.Contains(req.GetScopes(), oidc.ScopeOfflineAccess) && req.GetResponseType() == oidc.ResponseTypeCode && ValidateGrantType(client, oidc.GrantTypeRefreshToken)
	case TokenExchangeRequest:
		return req.GetRequestedTokenType() == oidc.RefreshTokenType
	case RefreshTokenRequest:
		return true
	default:
		return false
	}
}

Batch search results:

4.AuthStorage interface naming convention issues,

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

5. In the CreateAccessToken interface, it is necessary to use assertion to determine the different requests of TokenRequest in order to obtain ClientID and other information. Is it possible to split some interfaces here to achieve different scenarios?
6. There is also an assertion issue in the CreateAccessAndRefreshTokens interface. Meanwhile, when the request type is RefreshTokenRequest, the detailed information of the RefreshToken has been obtained in the RefreshTokenRequestByRefreshToken method, but no verification has been done. Instead, the RefreshToken is passed to the CreateAccessAndRefreshTokens interface.
7. In the RevokeToken interface, the token_type_hint field can be used to determine whether to revoke an access_token, refresh_token, or other tokens. However, the RevokeToken interface also provides a tokenIDOrToken field, which needs to be processed through assertion.

There may be some misunderstandings in my problem description, which the author can ignore.

[fforootd]

Hi @xslasd appreciate you feedback.

I am sure @muhlemmer and @livio-a find this helpful to improve zitadel/oidc.

[muhlemmer]

Thanks for the feedback @xslasd.

Insufficient usage examples and incomplete documentation. I spend a lot of time understanding functions by reading source code.

We currently have a help-wanted issue open for this: #145. My personal comments over there were from way before I joined the company, and after I did the focus shifted to urgently needed refactoring and implementing new features. So outside help is still welcome!

In this project, the third-party library github.com/gorilla/mux was used and we hope to replace it with local net/HTTP. This would allow for better integration into existing frameworks without the need for Gorilla/Linux libraries in the current project.

We have since moved to go-chi/chi on the next branch, due to be released with v3. Standard net/http is too restrictive for our needs. #332

Some parts of this project use type assertions, and I think we can reduce some assertion judgments. for example:

The many assertions came in place because we use so many interface types. In Go, when you add a method to an interface, it is a breaking change. So many optional interfaces where added,with assertions in order to maintain backwards compatibility for all users. We have started an effort to refactor this. As you might see in v2, we've changed from interfaces to concrete struct types for Token Claims related types. And in the next days I'll work to the same for IntroSpection reponse: #313.

It would also be nice if we can somehow merge the storage interfaces, but I want to come up with some kind of plan to let certain parts optional. We have storage types we don't (fully) use ourselves in Zitadel and vica-versa for other package users. I'm currently thinking of a proposal, but it might take some time.

I've also tried to reach out to the community for suggestions in this discussion: #276 but it has been mostly quit here, except some discussions on discord. Feel welcome to join the discussion anywhere.

5. In the CreateAccessToken interface, it is necessary to use assertion to determine the different requests of TokenRequest in order to obtain ClientID and other information. Is it possible to split some interfaces here to achieve different scenarios?
6. There is also an assertion issue in the CreateAccessAndRefreshTokens interface. Meanwhile, when the request type is RefreshTokenRequest, the detailed information of the RefreshToken has been obtained in the RefreshTokenRequestByRefreshToken method, but no verification has been done. Instead, the RefreshToken is passed to the CreateAccessAndRefreshTokens interface.
7. In the RevokeToken interface, the token_type_hint field can be used to determine whether to revoke an access_token, refresh_token, or other tokens. However, the RevokeToken interface also provides a tokenIDOrToken field, which needs to be processed through assertion.

If there are concrete bugs, or if you wish for somethings to be improved, please create a specific issue for each topic, so we can keep the scope of discussion small.