Implementation of implicit grant type is not fully compliant with RFC 6749

[PrieJos]

Dear aioauth Team,

I would like to bring into your attention that the implementation of the Implicit Grant Type in aioauth is not fully compliant with the specification described in RFC 6749. In particular, this spec in section 4.2.2 says:

The authorization server MUST NOT issue a refresh token.

However, your implementation is returning a refresh token as part of the Access Token Response described in this section. In particular, the fragments attached to the client's redirect URI is including a refresh token like this (I am omitting the real tokens in the next output):

'http://localhost/0123456789/token#state=mYegmEQQMgQ&expires_in=3600&refresh_token_expires_in=86400&access_token=xyz&refresh_token=abc&scope=&token_type=Bearer'

Expected Result

The redirection URL should not include the refresh token fields as stated by the RFC 6749 like this:

'http://localhost/0123456789/token#state=mYegmEQQMgQ&expires_in=3600&access_token=xyz&scope=&token_type=Bearer'

Actual Result

See the example above that was taken out of one test execution.

Reproduction Steps

The next code excerpt was taken from my pytest cases:

class TestImplicitGrant:
    """Test cases for implicit grant flow."""

    def test_complete_flow(
        self,
        fastplay_client_factory: t.Callable[..., "TestClient"],
        client_info: ClientInfoTuple,
        request_state: str,
        http_basic: "HTTPBasicFixture",
        token_type: str,
    ) -> None:
        """Test a successful complete authorization code flow."""
        # Create test client
        fastplay_client = fastplay_client_factory(
            FASTPLAY_ACCESS_TOKEN_TYPE=token_type,
            FASTPLAY_RFRESH_TOKEN_TYPE=token_type,
        )

        # Authorization request
        authorization_response = fastplay_client.get(
            "/auth/oauth2/authorize",
            params={
                "response_type": "token",
                "client_id": client_info.client_id,
                "redirect_uri": client_info.redirect_uri,
                "scope": "",
                "state": request_state,
            },
            auth=(http_basic.username, http_basic.password),
        )

        # Token response check
        assert authorization_response.status_code == HTTPStatus.FOUND
        assert authorization_response.next_request is not None
        redirect_url = authorization_response.next_request.url
        assert str(redirect_url).startswith(client_info.redirect_uri)
        assert len(redirect_url.fragment) > 0
        redirect_fragments: dict[str, str] = dict(
            urllib.parse.parse_qsl(
                redirect_url.fragment,
                keep_blank_values=True,
            ),
        )
        assert redirect_fragments["state"] == request_state
        assert redirect_fragments["scope"] == ""
        assert redirect_fragments["token_type"].lower() == "bearer"
        assert len(redirect_fragments["access_token"]) > 0
        assert (
            "refresh_token" not in redirect_fragments
            or len(redirect_fragments["refresh_token"]) == 0
        )

System Information

Python Version (3.x): 3.12
aioauth Version (0.x): 1.6.0
Framework (FastAPI / starlette, aiohttp, sanic, etc.)
- fastapi 0.112.0
- starlette 0.37.2

[aliev]

Thanks for spotting this issue! I'll definitely look into it. In the meantime, if you already have a solution in mind, feel free to create a pull request.

[PrieJos]

Hi @aliev

Thanks for spotting this issue! I'll definitely look into it. In the meantime, if you already have a solution in mind, feel free to create a pull request.

No, really, thanks for your great work! I created the PR #99 that indeed contains the proposed solutions for this issue and the issue #97 that I also opened a couple days ago.

Cheers Jose M. Prieto

[PrieJos]

Hi @aliev

I created the new PR (#108) as a final proposal for this issue. Please check it out and let me know if you have any questions.

Cheers Jose M. Prieto

[aliev]

Hi @PrieJos

I approved and merged #108, but despite having a feature flag, it is still breaking the backward compatibility because the signatures in the dataclasses and storage functions have changed.

I’ll think about how this can be fixed before including it in the minor version.