Page MenuHomePhabricator

test_identity test of oauth_tests.TestOauthLoginManager fails on wpbeta
Closed, ResolvedPublicBUG REPORT

Description

test_identity test of oauth_tests.TestOauthLoginManager fails on wpbeta since today.

======================================================================
FAIL: test_identity (tests.oauth_tests.TestOauthLoginManager)
Test identity.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/pywikibot/pywikibot/tests/oauth_tests.py", line 118, in test_identity
    self.assertIsInstance(login_manager.identity, dict)
AssertionError: None is not an instance of <class 'dict'>

----------------------------------------------------------------------

The underlying code snippet is:

@property
def identity(self) -> dict[str, Any] | None:
    """Get identifying information about a user via an authorized token.

    .. versionchanged:: 9.6
       *leeway* parameter for ``mwoauth.identify`` function was
       increased to 30.0 seconds.
    """
    if self.access_token is None:
        pywikibot.error('Access token not set')
        return None

    consumer_token = mwoauth.ConsumerToken(*self.consumer_token)
    access_token = mwoauth.AccessToken(*self.access_token)
    try:
        identity = mwoauth.identify(self.site.base_url(self.site.path()),
                                    consumer_token,
                                    access_token,
                                    leeway=30.0)
    except Exception as e:
        pywikibot.error(e)
    else:
        return identity

    return None

Event Timeline

Xqt triaged this task as High priority.Oct 29 2025, 5:06 PM
Xqt changed the subtype of this task from "Task" to "Bug Report".
Xqt moved this task from Backlog to Test failures on the Pywikibot-tests board.
bd808 subscribed.

Swapping Beta-Cluster-Infrastructure for Beta-Cluster-reproducible. Please do bring the Beta-Cluster-Infrastructure tag back if you can identify that the new test failure is because of a deployment or configuration issue specific to the Beta Cluster hosts.

Change #1199990 had a related patch set uploaded (by Xqt; author: Xqt):

[pywikibot/core@master] Test: Extract error message for T408721

https://gerrit.wikimedia.org/r/1199990

Change #1199990 merged by jenkins-bot:

[pywikibot/core@master] Test: Extract error message for T408721

https://gerrit.wikimedia.org/r/1199990

@bd808: I got the following error:
Unexpected issuer meta.wikimedia.beta.wmcloud.org, expected zh.wikipedia.beta.wmcloud.org
Unexpected issuer meta.wikimedia.beta.wmcloud.org, expected en.wikipedia.beta.wmcloud.org

The corresponding mwoauth code is

issuer = urlparse(identity['iss']).netloc
expected_domain = urlparse(mw_uri).netloc
if not issuer == expected_domain:
    raise OAuthException(
        "Unexpected issuer " +
        "{0}, expected {1}".format(issuer, expected_domain))

The expected domain is set from given host of the wiki. There is no other domain for the issuer found via siteinfo. Any idea how to get the issuer for beta clusters?

I think the issuer is always going to be meta because the wiki you request the auth from is always meta, isn't it?

I think the issuer is always going to be meta because the wiki you request the auth from is always meta, isn't it?

I guess I need to see more context of how the mwoauth library is being used in this case. I could be misunderstanding what is happening here.

This comment was removed by Xqt.

The mwoauth.identify function is used in login script to verify that the expected user is logged in (and by tests btw). See login_one_site and _oauth_login methods. They were introduced with T102602 (rPWBC8624ddec):

def _oauth_login(site) -> None:
    consumer_key, consumer_secret = _get_consumer_token(site)
    login_manager = OauthLoginManager(consumer_secret, site, consumer_key)
    login_manager.login()
    identity = login_manager.identity
    if identity is None:
        pywikibot.error(f'Invalid OAuth info for {site}.')
    elif site.username() != identity['username']:
        pywikibot.error(
            'Logged in on {site} via OAuth as {wrong}, but expect as {right}'
            .format(site=site,
                    wrong=identity['username'], right=site.username()))
    else:
        oauth_token = login_manager.consumer_token + login_manager.access_token
        pywikibot.info(
            f'Logged in on {site} as {site.username()} via OAuth consumer '
            f'{consumer_key}\nNOTE: To use OAuth, you need to copy the'
            ' following line to your user config file:\n'
            f'authenticate[{site.hostname()!r}] = {oauth_token}'
        )

The change was meant for OAuth 2 access tokens (which weren't very consistent in what issuer was used, anyway), they generally don't share code with the OAuth 1 /identify tokens mwoauth cares about but apparently issuer generation is one of the few bits that's shared (they both use UserStatementProvider). I guess we should just revert that; conceptually I think meta as issuer makes more sense but OAuth 1 is a legacy protocol and we should probably avoid changes to it.

Probably should include the OAuth 2 changes in Tech News, even if it's unlikely anyone is checking the issuer there (the signing key is not public so it would be very pointless).

I would like to avoid hardcoding the issuer. The issuer could be stored in the family file, but for beta famileis, these are primarily generated from siteinfo via a script. However, the issuer information is missing there.

A possible and effective approach would otherwise be to extract the issuer from the mwoauth error message and try again. What do you think about it?

The change was meant for OAuth 2 access tokens (which weren't very consistent in what issuer was used, anyway), they generally don't share code with the OAuth 1 /identify tokens mwoauth cares about but apparently issuer generation is one of the few bits that's shared (they both use UserStatementProvider). I guess we should just revert that; conceptually I think meta as issuer makes more sense but OAuth 1 is a legacy protocol and we should probably avoid changes to it.

I don't have strong opinions one way or another about the payload issuer value, but I very much agree that a breaking change to the OAuth 1a legacy workflow should be avoided as much as is reasonably possible. If the mwoauth implementation breaks an unspecified but large number of tools will break. Even if the library is updated and pywikibot is updated to require the newer library there will be a very long tail of Python tools that will need to be contacted to force library upgrades (all pywikibot tools included).

Change #1200329 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/OAuth@master] Use canonical server as OAuth 1 /identify endpoint JWT issuer

https://gerrit.wikimedia.org/r/1200329

[OAuth 1 and OAuth 2] both use UserStatementProvider

In fact, they both use UserStatementProvider::getUserProfile() but only OAuth 1 uses UserStatementProvider::getUserStatement() which is the one that sets the issuer. I was confused about that.

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/OAuth/+/1198714 does change UserStatementProvider::getUserProfile() though and that would probably also break OAuth 1 clients so we need to fix that as well.

Change #1200329 merged by jenkins-bot:

[mediawiki/extensions/OAuth@master] Use canonical server as OAuth 1 /identify endpoint JWT issuer

https://gerrit.wikimedia.org/r/1200329

Seems like 1.45 was branched just before rEOAU8cb9142a4beb: Use central wiki as access token issuer was merged, so no need to worry about that.

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/OAuth/+/1198714 does change UserStatementProvider::getUserProfile() though and that would probably also break OAuth 1 clients so we need to fix that as well.

...it actually doesn't. UserStatementProvider is used in the JWT that's returned as the response body for the OAuth 1 Special:OAuth/identity endpoint and the OAuth 2 / OIDC oauth2/resource/profile endpoint, which is internally mostly unrelated from the JWT that's used as the access token. Only the access token is changed by the patch.

It's not great to have a JWT subject in the access token that's different from the JWT subject in the identity token, so maybe we should rethink that. But it won't break anything. So I think we are done here.