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

Fix token length which now appears to be above 600 #212

Closed
wants to merge 1 commit into from

Conversation

xelfer
Copy link
Contributor

@xelfer xelfer commented Aug 26, 2024

Issue #, if available:

#211

Description of changes:

Adjust SCIMEndpointAccessToken regex to allow more than 600 characters (raised to 620) as a freshly generated token by the AWS console is 603 characters.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ChrisPates
Copy link
Contributor

Change looks good, merging and running through test pipelines.

@ChrisPates ChrisPates self-requested a review August 28, 2024 08:37
@xelfer
Copy link
Contributor Author

xelfer commented Aug 28, 2024

@ChrisPates thanks! seems to be hitting a go version error?

@ChrisPates
Copy link
Contributor

ChrisPates commented Aug 28, 2024

Yeah, somethings got broken in the workflow.

I'll probably have to do work on the CICD pipeline as well.

I'm fixing that in the latest release and then I'll attempt to merge the PR again.

It appears to be related to go.lang 1.20.x -> 1.23.x change.

@xelfer
Copy link
Contributor Author

xelfer commented Aug 28, 2024

No problem, do you have an ETA at all? If it's longer than a few days I'll just deploy it outside of SAM for myself. Thanks!

@ChrisPates
Copy link
Contributor

Hopefully today/tomorrow,

I wouldn't bothered with SAM, I would use the official builds in SAR. A lot less hassle.

Chris

@xelfer
Copy link
Contributor Author

xelfer commented Aug 28, 2024

awesome, thank you!

@ChrisPates
Copy link
Contributor

Okay, the workflows are working, can you update your fork and PR from master, and we can see whether we can merge.

Copy link
Contributor

@ChrisPates ChrisPates left a comment

Choose a reason for hiding this comment

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

Ideally this change would be based on the published regex from the service team but I can't it currently.

@xelfer xelfer closed this by deleting the head repository Aug 28, 2024
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.

2 participants