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

JWT is not well formed, there are no dots #1111

Open
RogerSelwyn opened this issue Nov 8, 2024 · 74 comments
Open

JWT is not well formed, there are no dots #1111

RogerSelwyn opened this issue Nov 8, 2024 · 74 comments

Comments

@RogerSelwyn
Copy link
Contributor

I have a few people who use the Home Assistant O365 integration that are getting the errors as shown below (they are encapsulated in HA login, but you can see what is being surfaced by O365). I've only seen reports in the last few months, but I have no idea what is causing them. Unfortunately I don't get the problem myself (or just don't notice the error in my logs, so I don't really know what triggers it).

Any ideas as to what is causing the problem? The integration sets up a FileSystemTokenBackend, which it uses I the Account setup call, beyond that it leaves token management to O365 (apart from reading the token once to check granted permissions). Code as below:

def _try_authentication(perms, credentials, main_resource):
    _LOGGER.debug("Setup token")
    token_backend = FileSystemTokenBackend(
        token_path=perms.token_path,
        token_filename=perms.token_filename,
    )
    _LOGGER.debug("Setup account")
    account = Account(
        credentials,
        token_backend=token_backend,
        timezone=CONST_UTC_TIMEZONE,
        main_resource=main_resource,
    )

    try:
        return account, account.is_authenticated

    except json.decoder.JSONDecodeError:
        return account, False
Logger: custom_components.o365.calendar
Quelle: custom_components/o365/calendar.py:655
Integration: Office 365 (Dokumentation, Probleme)
Erstmals aufgetreten: 08:48:54 (1 Vorkommnisse)
Zuletzt protokolliert: 08:48:54


Error getting calendar events - 401 Client Error: Unauthorized for url: https://graph.microsoft.com/v1.0/me/calendars/AQMkADAwATNiZmYAZC04M2ZhLTkwNDYtMDACLTAwCgBGAAADxUpIqt1PQ02qEe37a0cywQcAe9JyutQAp0a3sg19Hw9tFgAAAgEGAAAAe9JyutQAp0a3sg19Hw9tFgAF1RvVAwAAAA==/calendarView?%24top=999&startDateTime=2024-11-08T07%3A48%3A44.399251%2B00%3A00&endDateTime=2024-11-09T07%3A48%3A44.399274%2B00%3A00&%24select=sensitivity%2Cstart%2Ccategories%2Cbody%2CisAllDay%2Csubject%2Cend%2Clocation%2CseriesMasterId%2Cattendees%2CshowAs | Error Message: IDX14100: JWT is not well formed, there are no dots (.). The token needs to be in JWS or JWE Compact Serialization Format. (JWS): 'EncodedHeader.EncodedPayload.EncodedSignature'. (JWE): 'EncodedProtectedHeader.EncodedEncryptedKey.EncodedInitializationVector.EncodedCiphertext.EncodedAuthenticationTag'.
Logger: O365.connection
Quelle: /usr/local/lib/python3.12/site-packages/O365/connection.py:838
Erstmals aufgetreten: 08:48:54 (1 Vorkommnisse)
Zuletzt protokolliert: 08:48:54

Client Error: 401 Client Error: Unauthorized for url: https://graph.microsoft.com/v1.0/me/calendars/AQMkADAwATNiZmYAZC04M2ZhLTkwNDYtMDACLTAwCgBGAAADxUpIqt1PQ02qEe37a0cywQcAe9JyutQAp0a3sg19Hw9tFgAAAgEGAAAAe9JyutQAp0a3sg19Hw9tFgAF1RvVAwAAAA==/calendarView?%24top=999&startDateTime=2024-11-08T07%3A48%3A44.399251%2B00%3A00&endDateTime=2024-11-09T07%3A48%3A44.399274%2B00%3A00&%24select=sensitivity%2Cstart%2Ccategories%2Cbody%2CisAllDay%2Csubject%2Cend%2Clocation%2CseriesMasterId%2Cattendees%2CshowAs | Error Message: IDX14100: JWT is not well formed, there are no dots (.). The token needs to be in JWS or JWE Compact Serialization Format. (JWS): 'EncodedHeader.EncodedPayload.EncodedSignature'. (JWE): 'EncodedProtectedHeader.EncodedEncryptedKey.EncodedInitializationVector.EncodedCiphertext.EncodedAuthenticationTag'. | Error Code:
The user has checked their access token. There are no dots inside. They've also checked it with jwt.io and get the following error:

Error: Looks like your JWT Header is not encoded correclty using base64url (https://tools.ietf.org/html/rfc4648#section-5). Note that padding ("=") must be omitted as per https://tools.ietf.org/html/rfc7515'section-2
The error does not occur permanently, but only occasionally. It then seems to heal itself, since the synchronization of tasks and calendar generally works.

@alejcas
Copy link
Member

alejcas commented Nov 8, 2024

Hi, I’ll have some time next week to investigate. I’ll report back. Never happened this to me before nor anyone reported this earlier.

@RogerSelwyn
Copy link
Contributor Author

I’ve previously seen this issue logged, similar error, different situation I think - #998

@alejcas
Copy link
Member

alejcas commented Nov 15, 2024

The problem seems to be that oauthlib is no longer caching the correct error codes MS Graph gives and thus giving apps error codes like 401 (unauthorised) when it should raise TokenExpiredError.
I have already exposed this but I don't have the time not the expertise to go into this.

From my end this is difficult to solve as O365 don't know if a 401 is returned because the token is expired and oauthlib didn't do it's work raising the TokenExpiredError or it's actually a unauthorised error that is correct.

So, either anyone solves this inside oauthlib (which I think should be very difficult) or O365 drops it's custom auth methods to use msal.

I would go wih msal, but this is a BIG change that touches many 'moving parts'.

@RogerSelwyn
Copy link
Contributor Author

I’m not sure I have the knowledge to rework the auth method, but I can take a look when I get back home next week.

In the interim, presumably I can trap the errors in my integration and trigger a token refresh?

@alejcas
Copy link
Member

alejcas commented Nov 15, 2024

The current quick solution involves the following (other users reported this several times here):

  1. Manually catch the exception
  2. Call account.connection.refresh_token()
  3. Try the request again

@RogerSelwyn
Copy link
Contributor Author

Thanks

@alejcas
Copy link
Member

alejcas commented Nov 15, 2024

And please remember that the refresh should ask before to the token backed:

should_refresh_token = self.token_backend.should_refresh_token(self)
if should_refresh_token is True:
    # The backend has checked that we can refresh the token
    if self.refresh_token() is False:
        raise RuntimeError('Token Refresh Operation not working')

Although should_refresh_token is barely used by anyone...

@alejcas
Copy link
Member

alejcas commented Nov 15, 2024

I have analysed what it means to implement authentication with MSAL.

  • Authentication:
  1. Modify methods from Connection object: request_token, refresh_token and get_authorization_url
  2. Modify methods from Account object: authenticate
  3. Modify object Token: msal tokens are somehow different. I need to figure out how to adapt the token methods so O365 can know when a token is expired.
  4. Modify methods: _internal_requestand oauth_request
  • Readme: Modify the readme to show the change. Many methods change and auth is done differently

  • Optional changes: Configure a msal TokenCache to work with O365 Token Backends

I have been able to authenticate and use a O365 with tokens retrieved using msal.

I think I will drop custom auth in favor of msal as soon as possible
Many work to do and testing...

@RogerSelwyn
Copy link
Contributor Author

RogerSelwyn commented Nov 15, 2024

That would be brilliant. I'd be happy to test as soon as you have something (preferably not before the 23rd, if it requires code change at my end).

@alejcas
Copy link
Member

alejcas commented Nov 15, 2024

I will be working in the msal branch

https://github.com/O365/python-o365/tree/msal

@RogerSelwyn
Copy link
Contributor Author

Not sure on the notes you have written on the meal branch whether you have finished work and it is testable, our whether it is WUP. I see you have note that 2.0.38 is the last 2.0 release, which suggests you will go to 2.1/3.0 for new authentication model.

As soon as you are ready I can test.

@alejcas
Copy link
Member

alejcas commented Nov 28, 2024

Not sure on the notes you have written on the meal branch whether you have finished work and it is testable, our whether it is WUP. I see you have note that 2.0.38 is the last 2.0 release, which suggests you will go to 2.1/3.0 for new authentication model.

As soon as you are ready I can test.

It is difficult to integrate between both without breaking changes. I really don't know if it will be possible without a huge amount of efford

@RogerSelwyn
Copy link
Contributor Author

It is difficult to integrate between both without breaking changes. I really don't know if it will be possible without a huge amount of effort

No problem. If it isn't possible, then I understand. If you can do it, I think it is the right thing to do. I'm buried at the moment, but I can also take a look, probably in the New Year. However, I suspect if you can't do it, no-one can. I'd be coming at it with inexperienced eyes with regards OAUTH.

@dreamworks
Copy link

dreamworks commented Dec 12, 2024

grafik

This looks weird to me. Any of those parms (jwt, top, startDateTime, endDateDateTime, … ) should be posted and not included in the url. I am not sure if this error has anything to do with the JWT token.

The dot within the StartDateTime looks even more strange. Could it be the case that it's just a bad error message from the backend and is not related to the token itself?

@alejcas
Copy link
Member

alejcas commented Dec 12, 2024

grafik

This looks weird to me. Any of those parms (jwt, top, startDateTime, endDateDateTime, … ) should be posted and not included in the url. I am not sure if this error has anything to do with the JWT token.

The dot within the StartDateTime looks even more strange. Could it be the case that it's just a bad error message from the backend and is not related to the token itself?

For me it's very difficult to test this. I don't see this error anywhere in my tests....
As reported in earlier comments the problem seems to be that oauthlib is no longer caching the correct error codes MS Graph reports (see earlier comments),

@RogerSelwyn I'm 75% done with the use of msal.
Unfortunately some breaking changes are needed
They are small in numbers but will hit hard to all users.

For example: users already authenticated will need to re-authenticate as the stored tokens are no longer valid in their current form.

In the msal branch you can currently do all except that the requests will stop working after 1 hour as I haven't yet changed the refresh token methods.

So:

  • you can use msal in all authentication forms (auth_flow_types). But I have only tested 'authorization'.
  • you can use token backends to load or save tokens (not compatible with already existing stored tokens)
  • you can use all the library pieces.
  • you CAN'T refresh a token once it's expired for the momment

@alejcas
Copy link
Member

alejcas commented Dec 12, 2024

Btw!
I'm now 100% sure that the problem with the authentication is that MS GRAPH is sending a completely different error message as before when the access token is expired.

When it's expired it returns:
Client Error: 401 Client Error: Unauthorized for url: XXXXXX | Error Message: IDX14100: JWT is not well formed, there are no dots (.).

This is nonsense... and is also a problem with msal use.
Anyway when a 401 error raises O365 will first try to refresh the token. If a 401 is raised again then it's a true Unauthorized response.

@RogerSelwyn
Copy link
Contributor Author

That sounds like amazing progress. For me it seems likely it will not be a major impact if people need to re-authenticate. Firstly I’m at a changeover point from one code set to a new one, so people are doing a new setup anyway. Secondly I’ve made the management of re-authentication simpler.

I’ll try to test with the code base when I get home, may not be until Wednesday.

@RogerSelwyn
Copy link
Contributor Author

Btw! I'm now 100% sure that the problem with the authentication is that MS GRAPH is sending a completely different error message as before when the access token is expired.

When it's expired it returns: Client Error: 401 Client Error: Unauthorized for url: XXXXXX | Error Message: IDX14100: JWT is not well formed, there are no dots (.).

This is nonsense... and is also a problem with msal use. Anyway when a 401 error raises O365 will first try to refresh the token. If a 401 is raised again then it's a true Unauthorized response.

Is this a change you are now including into the code? It is it already there?

@alejcas
Copy link
Member

alejcas commented Dec 12, 2024

Is this a change you are now including into the code? It is it already there?

I'm doing it atm. I will probably finish it tomorrow. Then I'll need some help running some tests. I will be glad if you can help me there. Thanks

@RogerSelwyn
Copy link
Contributor Author

I’m away, may be able to test Sunday/Monday.

@alejcas
Copy link
Member

alejcas commented Dec 13, 2024

Done!, It works and refreshes and everything as before.

I'll implement "multi-account" authentication next, as until now 1 account instance can only hold 1 authentication, Now it can hold as many as you want

@alejcas
Copy link
Member

alejcas commented Dec 14, 2024

I'll implement "multi-account" authentication next, as until now 1 account instance can only hold 1 authentication, Now it can hold as many as you want

Done as well.

After the testing I will release asap

@RogerSelwyn
Copy link
Contributor Author

When I try to authenticate I get this error. Do you no longer need offline_access?

ValueError: You cannot use any scope value that is reserved.
Your input: ['Calendars.ReadWrite', 'offline_access', 'User.Read']
The reserved list: ['offline_access', 'openid', 'profile']

@alejcas
Copy link
Member

alejcas commented Dec 15, 2024

When I try to authenticate I get this error. Do you no longer need offline_access?

ValueError: You cannot use any scope value that is reserved.
Your input: ['Calendars.ReadWrite', 'offline_access', 'User.Read']
The reserved list: ['offline_access', 'openid', 'profile']

Yes, remove it

@RogerSelwyn
Copy link
Contributor Author

If I remove offline_access, which I think is not needed any more I get this error.

  File "/HADev/Hassio/dev-config/custom_components/ms365_calendar/config_flow.py", line 158, in async_step_request_default
    errors = await self._async_validate_response(user_input)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/HADev/Hassio/dev-config/custom_components/ms365_calendar/config_flow.py", line 231, in _async_validate_response
    result = await self.hass.async_add_executor_job(
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/connection.py", line 587, in request_token
    result = self.msal_client.acquire_token_by_auth_code_flow(flow, auth_response=query_params_dict)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/msal/application.py", line 1096, in acquire_token_by_auth_code_flow
    auth_code_flow.pop("claims_challenge", None))),
    ^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'pop'

Do you need for me to pull something else out of that call?

@alejcas
Copy link
Member

alejcas commented Dec 15, 2024

What auth_flow_type are you using?

@RogerSelwyn
Copy link
Contributor Author

I don't pass any flow type to Account setup, so I believe it is "authorization".

@alejcas
Copy link
Member

alejcas commented Dec 15, 2024

so I believe it is "authorization".

Yes it is by default.

Mmm can you just give me the "simplified" code used until the error?

@RogerSelwyn
Copy link
Contributor Author

In the call at 587:

  • Flow - None
  • query_params_dict - dict of code, client-Info, state, session_state

@alejcas
Copy link
Member

alejcas commented Dec 15, 2024

Mmm ok, I think I know what’s happening there.
The new code loads the token from the backend at startup, and then will save the new token info in the backend. Therefore both token info will be present.

I’ll add a check to see if the token data is the old one and if it is will not load that token data in.

@RogerSelwyn
Copy link
Contributor Author

I'm getting this error after some time, so I'm guessing it is token refresh related. I'll try to dig into it.

Traceback (most recent call last):
  File "/workspaces/core/homeassistant/config_entries.py", line 729, in __async_setup_with_context
    result = await component.async_setup_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/HADev/Hassio/dev-config/custom_components/ms365_calendar/__init__.py", line 48, in async_setup_entry
    check_token = await _async_check_token(hass, account, entity_name)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/HADev/Hassio/dev-config/custom_components/ms365_calendar/__init__.py", line 108, in _async_check_token
    await hass.async_add_executor_job(account.get_current_user_data)
  File "/usr/local/lib/python3.12/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/account.py", line 144, in get_current_user_data
    return directory.get_current_user()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/directory.py", line 329, in get_current_user
    return self._get_user(url, query=query)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/directory.py", line 303, in _get_user
    response = self.con.get(url, params=params)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/connection.py", line 867, in get
    return self.oauth_request(url, 'get', params=params, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/connection.py", line 851, in oauth_request
    if self.refresh_token():
       ^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/connection.py", line 680, in refresh_token
    result = self.msal_client.acquire_token_silent_with_error(
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/msal/application.py", line 1416, in acquire_token_silent_with_error
    return _clean_up(self._acquire_token_silent_with_error(
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/msal/application.py", line 1429, in _acquire_token_silent_with_error
    assert isinstance(scopes, list), "Invalid parameter type"
           ^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Invalid parameter type

@RogerSelwyn
Copy link
Contributor Author

RogerSelwyn commented Dec 15, 2024

The error occurs at startup. The parameters in at line 680 are:

  • scopes=self.scope - was None
  • account=self.msal_client.get_accounts(username=self.current_username)[0] - which seems to be a populated dict of the user account

Not sure why scopes is empty.

My call in that triggered the error is account.get_current_user_data, this used to fail with InvalidClientError if the token was invalid, or the client secret had expired. Success indicated the token was OK.

Previous calls, were:

  • token_backend = FileSystemTokenBackend(token_path=perms.token_path, token_filename=perms.token_filename)
  • account = Account(credentials, token_backend=token_backend, timezone=CONST_UTC_TIMEZONE, main_resource=main_resource)
  • account.is_authenticated was True
  • account.get_current_user_data()

I can do an alternate call if needed.

EDIT: If I add scopes into the call to Account setup it seems to work. Didn't need to do this before, but it's not a problem.

@RogerSelwyn
Copy link
Contributor Author

Just to add, I've had to modify some of my tests because of the changes in your code, but they are running fine. Just the issue above around refreshing the token.

@alejcas
Copy link
Member

alejcas commented Dec 16, 2024

I've done the following:

  1. Removed offline_access as one of the default scopes
  2. Added a check under BaseTokenBackend.load_token so that when trying to load an old style token it will raise a value error
  3. Added BaseTokenBackend.get_token_scopes so you can get the scopes as a list

@alejcas
Copy link
Member

alejcas commented Dec 16, 2024

  • scopes=self.scope - was None

I don't know why self.scopes is None.

For me it's not None. It can't be None as scopes are required. It will raise an error if they are not set in the call to account.authenticate()

Can you please see where is connection.scopes None?
I mean I suppose is not None when you instantiate Account, but then where gets the value None?

@RogerSelwyn
Copy link
Contributor Author

I don't know why self.scopes is None.

For me it's not None. It can't be None as scopes are required. It will raise an error if they are not set in the call to account.authenticate()

Can you please see where is connection.scopes None? I mean I suppose is not None when you instantiate Account, but then where gets the value None?

There is no call to account.authenticate() in the sequence where authentication has previously occurred and it's working from a stored token, which it is now trying to refresh because it has expired.

So the scenario is, I have previously authenticated and retrieved my token, which is stored in a file location. My application is then re-started, I don't need to call account.authenticate(), because I give O365 the token to use. So the calls I make are:

  • FileSystemTokenBackend,
  • Account setup,
  • retrieve the value of is_authenticated (which is True because I have authenticated),
  • get_current_user_data - which triggers the token refresh, which fails

None of these calls require the scope to be passed in.

  • In Account setup, kwargs can be passed, Connection is setup passing in kwargs
  • In Connection, self.scopes is set to None because the default value on init is None (if you pass scope into account setup, then it is passed to Connection and scopes is set correctly).
  • Consequently at line 680 in Connection, self.scopes is passed as None to 'acquire_token_silent_with_error).

I had assumed that because the token is passed into Account setup, that the scope would be retrieved from there, but I can't see anything that would so that. I'll try to have my test environment running at the point at which it would refresh the token after it expiring during normal running, to see if I get the same error there.

I had anticipated that if I had passed in the token, it would retrieve the scopes from there. Not a major problem because I can pass them in at Account setup, but if it is mandatory, then it should be modified to make sure it is provided.

@RogerSelwyn
Copy link
Contributor Author

I'll try to have my test environment running at the point at which it would refresh the token after it expiring during normal running, to see if I get the same error there.

The same thing happens. So I guess you either need to pass scopes into Account setup, or being doing an account.authenticate(). I don't think you will do .authenticate() more than once (certainly not after every app startup), so scopes need to be passed into Account, then it will get a new access token using the refresh token.

@RogerSelwyn
Copy link
Contributor Author

I've done the following:

  1. Removed offline_access as one of the default scopes
  2. Added a check under BaseTokenBackend.load_token so that when trying to load an old style token it will raise a value error
  3. Added BaseTokenBackend.get_token_scopes so you can get the scopes as a list

I'll try these out over the next couple of days.

@alejcas
Copy link
Member

alejcas commented Dec 16, 2024

ok, but before this change you had to always pass the scopes anyway into the connection (with or without token stored).
Was not the case for you?

I can of course get the scopes stored from the token and then pass them into the auth calls anyway, but it wasn't working this way previously.

Although maybe python oauthlib was doing something under the hood I was not aware of...

I can change it so when there are no scopes defined, but there is a token stored we can get the scopes from the token, ignoring any scopes passed. So scopes are only needed when authenticating.

@RogerSelwyn
Copy link
Contributor Author

ok, but before this change you had to always pass the scopes anyway into the connection (with or without token stored). Was not the case for you?

No, I didn't pass scopes into Account setup before. But I can do easily.

I can of course get the scopes stored from the token and then pass them into the auth calls anyway, but it wasn't working this way previously.

Although maybe python oauthlib was doing something under the hood I was not aware of...

I can change it so when there are no scopes defined, but there is a token stored we can get the scopes from the token, ignoring any scopes passed. So scopes are only needed when authenticating.

Up to you, it is more changes for people using the library.

@alejcas
Copy link
Member

alejcas commented Dec 16, 2024

Up to you, it is more changes for people using the library.

I've done it.

Can you test it please?

Thanks a lot

@RogerSelwyn
Copy link
Contributor Author

  1. Removed offline_access as one of the default scopes

This change now causes the following error:

2024-12-16 19:10:36.736 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry new for ms365_calendar
Traceback (most recent call last):
  File "/workspaces/core/homeassistant/config_entries.py", line 729, in __async_setup_with_context
    result = await component.async_setup_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/HADev/Hassio/dev-config/custom_components/ms365_calendar/__init__.py", line 39, in async_setup_entry
    account, is_authenticated = await hass.async_add_executor_job(
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/HADev/Hassio/dev-config/custom_components/ms365_calendar/__init__.py", line 96, in _try_authentication
    account = Account(
              ^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/account.py", line 35, in __init__
    kwargs['scopes'] = self.protocol.get_scopes_for(scopes)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/connection.py", line 196, in get_scopes_for
    scopes.add(self.prefix_scope(scope))
               ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/connection.py", line 203, in prefix_scope
    if not scope.startswith(self.protocol_scope_prefix):
           ^^^^^^^^^^^^^^^^
AttributeError: 'tuple' object has no attribute 'startswith'

I was passing in scopes as 'User.Read Calendars.ReadWrite' which worked before. I have also tried ['User.Read', 'Calendars.ReadWrite'] which also failed.

@RogerSelwyn
Copy link
Contributor Author

I've done it.

Can you test it please?

Thanks a lot

Will do. Need to wait for token to expire.

@alejcas
Copy link
Member

alejcas commented Dec 16, 2024

  1. Removed offline_access as one of the default scopes

This change now causes the following error:

2024-12-16 19:10:36.736 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry new for ms365_calendar
Traceback (most recent call last):
  File "/workspaces/core/homeassistant/config_entries.py", line 729, in __async_setup_with_context
    result = await component.async_setup_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/HADev/Hassio/dev-config/custom_components/ms365_calendar/__init__.py", line 39, in async_setup_entry
    account, is_authenticated = await hass.async_add_executor_job(
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/HADev/Hassio/dev-config/custom_components/ms365_calendar/__init__.py", line 96, in _try_authentication
    account = Account(
              ^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/account.py", line 35, in __init__
    kwargs['scopes'] = self.protocol.get_scopes_for(scopes)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/connection.py", line 196, in get_scopes_for
    scopes.add(self.prefix_scope(scope))
               ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/connection.py", line 203, in prefix_scope
    if not scope.startswith(self.protocol_scope_prefix):
           ^^^^^^^^^^^^^^^^
AttributeError: 'tuple' object has no attribute 'startswith'

I was passing in scopes as 'User.Read Calendars.ReadWrite' which worked before. I have also tried ['User.Read', 'Calendars.ReadWrite'] which also failed.

??? Scope is now never a tuple. It was when offline_access was in it. Now I removed all checks of scope being a tuple.
I can't reproduce this.
Can you look what scope is before the error?

@alejcas
Copy link
Member

alejcas commented Dec 16, 2024

  1. Removed offline_access as one of the default scopes

This change now causes the following error:

2024-12-16 19:10:36.736 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry new for ms365_calendar
Traceback (most recent call last):
  File "/workspaces/core/homeassistant/config_entries.py", line 729, in __async_setup_with_context
    result = await component.async_setup_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/HADev/Hassio/dev-config/custom_components/ms365_calendar/__init__.py", line 39, in async_setup_entry
    account, is_authenticated = await hass.async_add_executor_job(
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/concurrent/futures/thread.py", line 58, in run
    result = self.fn(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/HADev/Hassio/dev-config/custom_components/ms365_calendar/__init__.py", line 96, in _try_authentication
    account = Account(
              ^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/account.py", line 35, in __init__
    kwargs['scopes'] = self.protocol.get_scopes_for(scopes)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/connection.py", line 196, in get_scopes_for
    scopes.add(self.prefix_scope(scope))
               ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/O365/connection.py", line 203, in prefix_scope
    if not scope.startswith(self.protocol_scope_prefix):
           ^^^^^^^^^^^^^^^^
AttributeError: 'tuple' object has no attribute 'startswith'

I was passing in scopes as 'User.Read Calendars.ReadWrite' which worked before. I have also tried ['User.Read', 'Calendars.ReadWrite'] which also failed.

??? Scope is now never a tuple. It was when offline_access was in it. Now I removed all checks of scope being a tuple. I can't reproduce this. Can you look what scope is before the error?

Found it and fix it!

Sorry

@RogerSelwyn
Copy link
Contributor Author

I have a busy day tomorrow, so may not get to do any testing. Should get some time Wednesday.

@alejcas
Copy link
Member

alejcas commented Dec 16, 2024

Whenever you can, thanks

@RogerSelwyn
Copy link
Contributor Author

I’ve tested most things. The method to retrieve the scopes is too complex to include at the right point in my code. The method isn’t complex, just providing the right info to it (username) at the point I need the info. So I haven’t tested it.

Token refresh seems to be working, based on me not supplying scopes to account setup, so you retrieving it seems to be working fine.

Next step for me would be to trial on my production environment, but could do with a pip package for that.

@RogerSelwyn
Copy link
Contributor Author

Actually I think I can test it in production without pip package. I’ll try in the next couple of days.

@alejcas
Copy link
Member

alejcas commented Dec 17, 2024

The method isn’t complex, just providing the right info to it (username) at the point I need the info. So I haven’t tested it.

Mind that passing the username is optional. Most users will only have 1 auth stored (1 user). If you don’t pass the username, the info retrieved will be the one for the first username found. Since most people will one have 1 auth this is completely transparent.

Nevertheless I will set a method inside account to retrieve all the users authenticated, but as I said this will only affect a small user base. Also mind that this is how msal works so I have to provide a username always and O356 does it in a way that is transparent for the users.

Also I have included this info in the readme of the msal branch.

@RogerSelwyn
Copy link
Contributor Author

RogerSelwyn commented Dec 17, 2024

I'll try it again, I think when I tried with no username I got nothing back, but it would have been before I did account setup. Been a long day today, so it won't be until tomorrow.

I tried it based on just FileSystemTokenBackend setup. But I was fighting another issue, so I didn't try too hard.

@RogerSelwyn
Copy link
Contributor Author

I've tested BaseTokenBackend.get_token_scopes, had to adapt my code to do account setup first, which in reality is more logical, so that is fine. I've also tested the ValueError from using an old token.

So I think I can test as much as I can. My full pytest suite tests using mocked ms graph api responses, but should be reasonably good. Also I've run in my dev environment for some hours and it seems to be handling refreshes fine.

Do you want me to test in my prod, before you release or are you happy with it?

@alejcas
Copy link
Member

alejcas commented Dec 18, 2024

I’m more worried for the people using credentials auth flow type.

I think we can release it as it is and iterate over.

@RogerSelwyn
Copy link
Contributor Author

I’m more worried for the people using credentials auth flow type.

Not sure I can meaningfully test that since I don't use it. I could possibly work out how to use it, might take a while though.

@RogerSelwyn
Copy link
Contributor Author

Do you have an idea as to when you will release?

@alejcas
Copy link
Member

alejcas commented Dec 20, 2024

Soon!

@RogerSelwyn
Copy link
Contributor Author

You have written this in the readme
It is highly recommended to add the "offline_access" permission. Otherwise the library will only have access to the user resources for 1 hour. See [Permissions and Scopes](#permissions-and-scopes).

I don't think you can add offline_access, if you do, you will get this error:

  File "/home/vscode/.local/ha-venv/lib/python3.13/site-packages/O365/connection.py", line 552, in get_authorization_url
    flow = self.msal_client.initiate_auth_code_flow(scopes=scopes, redirect_uri=redirect_uri)
  File "/home/vscode/.local/ha-venv/lib/python3.13/site-packages/msal/application.py", line 942, in initiate_auth_code_flow
    scope=self._decorate_scope(scopes),
          ~~~~~~~~~~~~~~~~~~~~^^^^^^^^
  File "/home/vscode/.local/ha-venv/lib/python3.13/site-packages/msal/application.py", line 691, in _decorate_scope
                raise ValueError(
    ...<2 lines>...
    The reserved list: {}""".format(list(scope_set), list(reserved_scope)))
ValueError: You cannot use any scope value that is reserved.
Your input: ['Calendars.ReadWrite', 'User.Read', 'offline_access']
The reserved list: ['openid', 'profile', 'offline_access']

@RogerSelwyn
Copy link
Contributor Author

RogerSelwyn commented Dec 20, 2024

retreive the state saved in auth_step_one - retreive should be retrieve
as it's asuming there is are no parallel connections running at once - should be assuming

@RogerSelwyn
Copy link
Contributor Author

I think I have a problem where if I try to retrieve a new token (because I can change scope news within the app while it is running), when I already have an existing one, then it returns the existing token. I think it returns it from the cache, because before I do the call to '.request_token` I've just added a process to delete the stored file token.

So steps:

  1. Setup account and ensure it is authenticated
  2. Re-specify scope and get_authorization_url
  3. Authenticate on the MS dialogues
  4. Delete existing file token - I newly added this to try to diagnose issue
  5. Provide the returned url into request_token
  6. New created file token has same scope as at step 1

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

No branches or pull requests

3 participants