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

Add required scope for token refresh when refreshing Microsoft OAuth2 token #637

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

Conversation

AMLSMike
Copy link

@AMLSMike AMLSMike commented Feb 7, 2025

Proposed change

Add the missing and required scope field when refreshing Microsoft OAuth2 token.

Type of change

1 - 🐞 bug 🐞

Additional information

I have been writing an MSGraphAPI extension to fetch/send emails. During development, I changed the scope field many times! Everything worked fine until the automatic token refresh started. Suddenly I lost access to fetch and send emails. Then I dumped the received token in the logs and used https://jwt.ms/, only to see that I lost all my entered MSGraph scopes and the scope contained Outlook stuff again - I removed those completely before, since MSGraphAPI doesn't use any of them.

I went to the index.pl?Action=AdminOAuth2TokenManagement page and clicked manually refresh. Everything was working fine again! Until the auto refreshed is made...

When hovering the refresh button, all my entered scopes are contained in the URL.

I have noticed that when the token gets auto-refreshed, the scope is not sent within the request. Microsoft then seems to use the first initiated scope with the client token. This seems to be a bug on Microsoft's part! But to be fair, they have written in their Authentication Guides that the scope field is required.

There are also a lot more required fields: tanant, which is currently not configurable in Znuny. But hey, it works somehow 😋

Checklist

  • The code change is tested and works locally.(❗)
  • There is no commented out code in this PR.(❕)
  • You improved or added new unit tests.(❕)
  • Local ZnunyCodePolicy passed.(❕)
  • Local UnitTests / Selenium passed.(❕)
  • GitHub workflow CI (UnitTests / Selenium) passed.(❗)

@dennykorsukewitz dennykorsukewitz self-assigned this Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants