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

Failing to add Microsoft Azure Account #12317

Open
plaf2000 opened this issue Jun 7, 2024 · 4 comments
Open

Failing to add Microsoft Azure Account #12317

plaf2000 opened this issue Jun 7, 2024 · 4 comments

Comments

@plaf2000
Copy link

plaf2000 commented Jun 7, 2024

Expected Behavior

Successfully sign up with Microsoft Azure.

Actual Behavior

Getting the following error:

Traceback (most recent call last):
  File "/usr/local/lib/python3.10/dist-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.10/dist-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python3.10/dist-packages/allauth/socialaccount/providers/oauth2/views.py", line 87, in view
    return self.dispatch(request, *args, **kwargs)
  File "/usr/local/lib/python3.10/dist-packages/allauth/socialaccount/providers/oauth2/views.py", line 160, in dispatch
    login = self.adapter.complete_login(
  File "/usr/src/geonode/./geonode/people/adapters.py", line 343, in complete_login
    login = self.get_provider().sociallogin_from_response(request, extra_data)
  File "/usr/local/lib/python3.10/dist-packages/allauth/socialaccount/providers/base/provider.py", line 70, in sociallogin_from_response
    raise ValueError(f"uid must be a string: {repr(uid)}")

Exception Type: ValueError at /account/geonode_openid_connect/login/callback/
Exception Value: uid must be a string: None

Steps to Reproduce the Problem

  1. Follow the instruction to add a Microsoft Azure login described at https://docs.geonode.org/en/master/advanced/social/.
  2. Try to sign up using the Microsoft Azure option.

Specifications

  • GeoNode version: 4.3.x
  • Installation type (vanilla, geonode-project): geonode 4.3 as image, custom apps and overrides loaded as volumes.
  • Installation method (manual, docker): docker
  • Platform: ubuntu-22.04 (docker)

Where the Problem Lies

The 'unique_name' key specified by the UID_FIELD Azure's provider settings variable (SOCIALACCOUNT_PROVIDERS_DEFS['azure']['UID_FIELD']) is not in the response body from https://graph.microsoft.com/v1.0/me. The attribute that should be considered is 'id' instead: https://learn.microsoft.com/en-us/graph/api/user-get?view=graph-rest-1.0&tabs=http.

Proposed fix(es)

In order to fix the issue, it is enough to change the UID_FIELD to 'id'. This can therefore also be easly done by a settings override, but, probably, this should be the default field defined in settings.py.
To keep the backward compatibility with older API versions using the 'unique_name' field (is that so?), the following change could be taken into account as well:

# File geonode/people/socialaccount/providers/geonode_openid_connect/provider.py

def extract_uid(self, data):
        _uid_field = getattr(settings, "SOCIALACCOUNT_PROVIDERS", {}).get(PROVIDER_ID, {}).get("UID_FIELD", None)
        if _uid_field and _uid_field in data: # Checks that data has the defined field
            return data.get(_uid_field)
        else:
            return data.get("uid", data.get("sub", data.get("id")))

In that case it wouldn't be necessary to change the UID_FIELD value.

@mattiagiupponi
Copy link
Contributor

Hi @plaf2000
is an open source project, feel free to open a PR with the proposed fixes. Please dont forget to add some test too

@plaf2000
Copy link
Author

Hi @mattiagiupponi and thank you for your answer.
I will do that, but first I would like to know, if possible, the origin of the attribute 'unique_name': does that come from older versions of the API?

Thank you in advance

@plaf2000
Copy link
Author

@mattiagiupponi Some other questions regarding testing. I see that at the moment the only social account tested is Google.
Therefore:

  1. Would I need to write tests for Azure account from scratch?
  2. If so, is there a fake/test account to use to call the Microsoft API?
  3. If not (to the 2. question), should I simulate some fake API responses in the tests (e.g. how they do on django-allauth for the microsoft provider)?

@mattiagiupponi
Copy link
Contributor

  1. Would I need to write tests for Azure account from scratch?

If are not there, it would be nice

  1. If not (to the 2. question), should I simulate some fake API responses in the tests (e.g. how they do on django-allauth for the microsoft provider)?

Yes i guess mocking the responses is the best approach now

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

2 participants