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

Tycho sessionid #353

Merged
merged 4 commits into from
Jul 17, 2024
Merged

Tycho sessionid #353

merged 4 commits into from
Jul 17, 2024

Conversation

frostyfan109
Copy link
Contributor

Modify Tycho to allow encoding arbitrary environmental variables into app pods. We now encode sessionid as ACCESS_TOKEN and grader API URL as GRADER_API_URL (if the setting in appstore is set) into pod environments.

@frostyfan109 frostyfan109 changed the base branch from master to develop May 23, 2024 20:43
@joshua-seals
Copy link
Collaborator

@frostyfan109 What is the big picture here for this augmentation, for my own understanding?

from string import ascii_letters, digits, punctuation

UserModel = get_user_model()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looked odd due to multiple runs through of 256, so I asked cGPT to optimize.

Characters to use for the token

characters = string.ascii_letters + string.digits

# Generate a token and check for uniqueness
while True:
    token = ''.join(secrets.choice(characters) for _ in range(256))
    if not UserIdentityToken.objects.filter(token=token).exists():
        return token

Copy link
Contributor Author

@frostyfan109 frostyfan109 May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well we could just use the := operator but I'm not sure what versions of python we need to support in Appstore

@frostyfan109
Copy link
Contributor Author

@frostyfan109 What is the big picture here for this augmentation, for my own understanding?

This a change for Eduhelx so that we can embed a secret into apps so they can authenticate with appstore without embedding the cookie directly. I'm not sure this is the correct approach need to run it by a couple others.

@joshua-seals
Copy link
Collaborator

@frostyfan109 What is the big picture here for this augmentation, for my own understanding?

This a change for Eduhelx so that we can embed a secret into apps so they can authenticate with appstore without embedding the cookie directly. I'm not sure this is the correct approach need to run it by a couple others.

Ok that makes sense. Does this enable refreshes to the token now?

If no, may not be a big deal now, but the other potential option is having a sidecar that handles traffic for each app. The sidecar would do the session refresh and all that jazz and is easier to control (and the same) across all the differing applications we provide.
This is a "Mesh-ish" sort of capability and would also come with benefits of encryption between all endpoints if we so chose.

@frostyfan109
Copy link
Contributor Author

@frostyfan109 What is the big picture here for this augmentation, for my own understanding?

This a change for Eduhelx so that we can embed a secret into apps so they can authenticate with appstore without embedding the cookie directly. I'm not sure this is the correct approach need to run it by a couple others.

Ok that makes sense. Does this enable refreshes to the token now?

If no, may not be a big deal now, but the other potential option is having a sidecar that handles traffic for each app. The sidecar would do the session refresh and all that jazz and is easier to control (and the same) across all the differing applications we provide. This is a "Mesh-ish" sort of capability and would also come with benefits of encryption between all endpoints if we so chose.

No refresh right now, the tokens just live for the lifetime of the app. This isn't intended to be a permanent solution, but if it does become one I like your idea.

@Hoid Hoid merged commit 7137802 into develop Jul 17, 2024
8 of 9 checks passed
@Hoid Hoid deleted the tycho-sessionid branch July 17, 2024 16:02
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

Successfully merging this pull request may close these issues.

3 participants