Doorkeeper shouldn't generate a secret for public clients

[ThisIsMissEm]

Currently doorkeeper's Application model always generates a secret value, even when the client type should not have a client secret generated (i.e., public clients). This means we're storing a secret for applications even when a secret value is completely unneeded

I think the model & migrations likely need to be changed to make oauth_application.secret nullable, and only generate a secret if app.confidential?

The Doorkeeper::Application.by_uid_and_secret() method does do the right thing and not bother with secret validation if the client is public.

However, I'm not sure if POST /oauth/token will explicitly reject the request if client_secret is presented despite the client being public. It seems this happens more through a side effect of by_uid_and_secret()'s logic.

This would save significant database resources for applications such as Mastodon, where we've a significant number of applications, due to using a form of Dynamic Client Registration. (i.e., the application count is in the tens or hundreds of thousands, if not millions).

[nbulaj]

Thanks @ThisIsMissEm , agree, that would be great to change. Better to arrange API as well 👍 Would you like to propose a MR? Not sure when I'll be free to check it

[nbulaj]

Prepared a draft MR @ThisIsMissEm - #1727
If you have a chance to check - will be thankful 🤝

Basically API should already consider public/private clients when we introduced such functionality so I don't think we need to do here anything.

Existing applications have to remove not null constraint. Maybe I missed something more?