Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(auth): add support for device code grant flow #5680

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Nerixyz
Copy link
Contributor

@Nerixyz Nerixyz commented Oct 27, 2024

This PR adds support for Twitch's Device code grant flow (DCF). This makes it possible to (effectively) use tokens for much longer without requiring the user to reauthenticate (assuming the scopes didn't change).

Currently, this uses a client-id I created (THIS MUST BE CHANGED BEFORE A MERGE). Because we use the public client type, the client-id is limited to DCF-only (as far as I understand).

(effectively) closes #5169.

Here's a cool video I took in February when I started this (hasn't changed much):

firefox_2024-02-11_12-24-45.mp4

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/providers/twitch/TwitchAccount.hpp Show resolved Hide resolved
errors.append("Missing user ID");
}
if (username.isEmpty())
if (errors.length() > 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty]

Suggested change
if (errors.length() > 0)
if (!errors.empty())
Additional context

.qtinstall/Qt/6.6.2/gcc_64/include/QtCore/qlist.h:681: method 'QList'::empty() defined here

    inline bool empty() const noexcept
                ^

@jupjohn
Copy link
Contributor

jupjohn commented Oct 30, 2024

Been running this for a few days and seems solid 👍

@iProdigy
Copy link
Contributor

Should we validate that stored tokens have all of DEVICE_AUTH_SCOPES? (or perhaps should be tackled in a separate PR)

@Nerixyz
Copy link
Contributor Author

Nerixyz commented Oct 30, 2024

Should we validate that stored tokens have all of DEVICE_AUTH_SCOPES? (or perhaps should be tackled in a separate PR)

Twitch does that when we refresh tokens (the last time I tested it). The error you get there is pretty bad, though (iirc, it's the same one you get when the token is invalid). Should be enough to add a message with a link to re-authenticate.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/providers/twitch/TwitchAccount.hpp Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/providers/twitch/TwitchAccount.hpp Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants