Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Session Handling and Token Invalidation #158

Closed
Vadimkin opened this issue Sep 19, 2023 · 11 comments
Closed

Session Handling and Token Invalidation #158

Vadimkin opened this issue Sep 19, 2023 · 11 comments

Comments

@Vadimkin
Copy link

Vadimkin commented Sep 19, 2023

Hello,

Recently, Bluesky added access tokens (#151), and right now, the library allows exporting a session string (client.export_session_string()). This string includes the handle, DID, their access token, and a refresh token.

Previously, the refresh token had been implemented with a 2-month lifetime (and then developer should generate another refresh token with client.login(login=..., password=...)), but now, the CreateSession function returns another refresh token with another 2-month lifetime.

In my codebase, it's something like this:

def app_init():
    # On app init
    client = Client()
    client.login(BLUESKY_USER_HANDLE, BLUESKY_USER_APP_PASSWORD)
    with open(BLUESKY_SECRET_FILE, "w") as f:
        f.write(client.export_session_string())

def post_message(text):
    session_string = "..."  # content from file
    client = Client()
    client.login(session_string=session_string)

    if client._access_jwt and client._should_refresh_session():
        client._refresh_and_set_session()
        # Update file with new session
        with open(BLUESKY_SECRET_FILE, "w") as f:
            f.write(client.export_session_string())

    # client.send_post(...)

My point here is that protected methods are being used to resolve this problem with autorefresh. Is it better to make them public (if the token needs to be invalidated in this way)?

My initial expectation is that methods like client.send_post()/client.com.atproto.repo.create_record()/etc will take care of token invalidation (if needed), but I receive an error, and it looks like invalidation doesn't work for those methods.

@MarshalX
Copy link
Owner

MarshalX commented Sep 19, 2023

Hi! But SDK cares about refreshing of access token using refresh token for a long time 🧐 was added here #27

The problem that I can understand that we must export session every 2 hours because of refreshed tokens 😢

Or do you want to say that auto-refresh logic was broken in SDK?

@MarshalX
Copy link
Owner

MarshalX commented Sep 19, 2023

I mean that all methods are checking if the session is expired and renewing it if necessary

@Vadimkin
Copy link
Author

Vadimkin commented Sep 19, 2023

I mean that all methods are checking if the session is expired and renewing it if necessary

Aah, thanks! I'll recheck my implementation – looks like something wrong with tokens on my end.

The problem that I can understand that we must export session every 2 hours because of refreshed tokens

Yeah, you're correct! I guess I can do something like this one:

def post_message(text):
    session_string = "..."  # content from file
    client = Client()
    client.login(session_string=session_string)

    client.send_post(...)

    if client.export_session_string() != previous_session_string:
        # export it again

But probably some more elegant way is needed 🤔

@MarshalX
Copy link
Owner

We definitely should save the session to persistent storage at the end of the script. But we also must catch all errors first to not exit without saving the exported session.

This doesn't fit well with envs, CI/CD secrets, and so on 🥲 sad

@MarshalX
Copy link
Owner

We can use the exported session for two months before updating it in persistent storage. Isn't it?

@Vadimkin
Copy link
Author

There is also an idea to add an option to store the exported session as a file (this file could be always updated when a new token is generated, so you don't have to catch all errors) when Client() is initialized. Also, looks like some user/password fallback is needed as well.

We can use the exported session for two months before updating it in persistent storage. Isn't it?

You're correct in general, but It is something strange happens there with token with expiration date 2 days ago
CleanShot 2023-09-19 at 22 46 58@2x
and refresh token with expiration in December:
CleanShot 2023-09-19 at 22 47 35@2x

This token raises an error on client.login(session_string=...):

atproto.exceptions.BadRequestError: Response(success=False, status_code=400, content=XrpcError(error='ExpiredToken', message='Token has been revoked'), headers=Headers({'date': 'Tue, 19 Sep 2023 19:48:13 GMT', 'content-type': 'application/json; charset=utf-8', 'content-length': '59', 'connection': 'keep-alive', 'x-powered-by': 'Express', 'access-control-allow-origin': '*', 'ratelimit-limit': '3000', 'ratelimit-remaining': '2999', 'ratelimit-reset': '1695153193', 'ratelimit-policy': '3000;w=300', 'vary': 'Accept-Encoding'}))

🤯🤯🤯

@Vadimkin
Copy link
Author

Vadimkin commented Sep 19, 2023

I'd assume that something is broken on bsky side with this token – with newly generated tokens everything works fine! I think we're good to use the same exported session for 2 months!

@MarshalX
Copy link
Owner

MarshalX commented Sep 19, 2023

I can add the ability to register callback on token refresh. And the user can implement storing this token in db or file or wherever

@MarshalX
Copy link
Owner

I'd assume that something is broken on bsky side with this token – with newly generated tokens everything works fine! I think we're good to use the same exported session for 2 months!

Didn't you changed password or remove app password?

@Vadimkin
Copy link
Author

Didn't you changed password or remove app password?

Nope. Furthermore, for a token that was generated yesterday, I'm getting the same "Token has been revoked" error. Password/handle haven't been changed, and I am still able to generate new tokens with the same password.

Token generated at 1695150790, exp=1695157990
Refresh token generated at 1695150790, exp=1702926790 (Dec, 18)

>>> client = Client()
>>> client.login(session_string=sec)
atproto.exceptions.BadRequestError: Response(success=False, status_code=400, content=XrpcError(error='ExpiredToken', message='Token has been revoked'), headers=Headers({'date': 'Wed, 20 Sep 2023 09:07:36 GMT', 'content-type': 'application/json; charset=utf-8', 'content-length': '59', 'connection': 'keep-alive', 'x-powered-by': 'Express', 'access-control-allow-origin': '*', 'ratelimit-limit': '3000', 'ratelimit-remaining': '2998', 'ratelimit-reset': '1695201121', 'ratelimit-policy': '3000;w=300', 'vary': 'Accept-Encoding'}))

...

>>> client._import_session_string(sec)
SessionString(...)
>>> client._should_refresh_session()
True
>>> client._refresh_and_set_session()
atproto.exceptions.BadRequestError: Response(success=False, status_code=400, content=XrpcError(error='ExpiredToken', message='Token has been revoked'), ...)

@MarshalX
Copy link
Owner

so... does it mean that when you refresh the session and get a new refresh token, the old one is revoked?

Repository owner locked and limited conversation to collaborators Sep 20, 2023
@MarshalX MarshalX converted this issue into discussion #161 Sep 20, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants