Skip to content

feat: Add support for device code grant #229

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

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

Conversation

SimonVanacco
Copy link

@SimonVanacco SimonVanacco commented Mar 25, 2025

This PR adds support for device code grants, which has been available for some time in oauth2-server

Ready to be tested, I'd love to get some feedback if you think some areas could be improved !

Design considerations

  • I've decided to let end-users handle the device code verification page on their own. This is to prevent having to require twig in the bundle itself and create more complexity to accommodate all use cases. An example controller has been added in the documentation
  • This does not support verification_uri_complete, only verification_uri
  • Logic for code approval is in DeviceCodeRepository::approveDeviceCode. I was unable to re-use the logic from oauth2-server as they fetch the code by "device_code" instead of by "user_code". Not sure if this is a mistake in the parent bundle or some use-case I don't understand, but it does not accommodate most people use-cases. I'm open to moving the logic somewhere else if you find a better place for it !
  • I've added some basic tests but this is not my strong suit : please let me know if I missed something :)

Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Can you rebase it? Here are some comments also

@@ -0,0 +1,76 @@
# Password grant handling
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Password grant handling
# Device grant handling


class DeviceCodeController extends AbstractController
{

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

['form' => $form]
);
}

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

Comment on lines +32 to +39
return $this->entityManager->createQueryBuilder()
->select('dc')
->from(DeviceCode::class, 'dc')
->where('dc.userCode = :code')
->setParameter('code', $code)
->getQuery()
->getOneOrNullResult();
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return $this->entityManager->createQueryBuilder()
->select('dc')
->from(DeviceCode::class, 'dc')
->where('dc.userCode = :code')
->setParameter('code', $code)
->getQuery()
->getOneOrNullResult();
}
return $this->entityManager->createQueryBuilder()
->select('dc')
->from(DeviceCode::class, 'dc')
->where('dc.userCode = :code')
->setParameter('code', $code)
->getQuery()
->getOneOrNullResult();
}

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.

2 participants