Details
-
Task
-
Resolution: Unresolved
-
P2: Important
-
None
-
None
-
None
-
-
21
-
Foundation Sprint 119, Foundation Sprint 120
Description
This task is a collection of things to do for Device Authorization Flow support after 6.9 feature freeze.
_q_authenticate is used to handle authenticationRequired(QAuthenticator*) signal. Role and need for that needs to be clarified with device flow (currently not implemented)- It would seem to me the current QAuthenticator usage in QOAuth2AuthorizationCodeFlow is wrong (doesn't set password), and likely not used either. To my understanding authorization servers
rely on credentials being provided proactively, and don't use 401 responses (but rather 400 Bad Request). Support for QAuthenticator can be added later if there turns out to be need for it, but
for now, users can use network request modification possibility to proactively set any necessary authentication things needed
- It would seem to me the current QAuthenticator usage in QOAuth2AuthorizationCodeFlow is wrong (doesn't set password), and likely not used either. To my understanding authorization servers
Both QOAuth2AuthorizationCodeFlow and QOAuth2DeviceAuthorizationFlow have 'accessTokenURL' property. The name should actually be "tokenURL" because there are several tokens: access token, refresh token, ID token. Therefore deprecate the QOAuth2AuthorizationCodeFlow::accessTokenUrl and create a "tokenUrl" in the baseclass (and remove the temporary QOAuth2DeviceAuthorizationFlow::accessTokenUrl)- This was discussed in review, but avoid too many deprecations in general, let's not
Qt 6.9 introduces setModifyTokenRequest. However with device flow this also includes the authorization request => name "tokenRequest" is too narrow- has been now renamed as setModifyNetworkRequest
- tst_oauth2 and tst_deviceflow have many things in common, but sharing them is not trivial, or more aptly put, results in messy autotest code with a lot of if/elses. However the test cases should be finegroomed if more can be shared. Possibly a new tst_authorizationcodeflow test should be introduced, and the tst_oauth2 should just focus on testing abstract oauth2 setters
error(), requestFailed(), and errorOccurred() handling. This is not strictly speaking a device flow issue only, but the error handling should be revisited. Ideally the "errorOccurred" would be the catch-all error signal- errorOccurred (originally error()) is now renamed to serverReportedErrorOccurred() to emphasize that it's about subset of all errors. requestFailed() is the catch-all error signal. This can be later renamed as errorOccurred(), and the requestFailed() deprecated.
- add token convenience expiration things to device flow class. They are initially introduced to authorization code-flow class only because device flow is not yet merged. But they should be taken into use in device flow class as well
- Update Qt OAuth2 Overview documentation. It's focused on then-only flow, authorization code flow. But the new flow should be taken into account as well
Attachments
Issue Links
- split from
-
QTBUG-130611 [OAuth] Support Device Authorization Flow
- Closed
Gerrit Reviews
For Gerrit Dashboard: QTBUG-130844 | ||||||
---|---|---|---|---|---|---|
# | Subject | Branch | Project | Status | CR | V |
606380,2 | Update OAuth2 overview doc to include network request modification | dev | qt/qtnetworkauth | Status: NEW | 0 | 0 |
606430,1 | Add token expiration convenience to Device Flow class | dev | qt/qtnetworkauth | Status: NEW | 0 | 0 |
606436,1 | Include device flow into OAuth2 overview documentation | dev | qt/qtnetworkauth | Status: NEW | 0 | 0 |