Skip to content

Remove CUCUMBER_PUBLISH_TOKEN validation #2122

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

Closed
aslakhellesoy opened this issue Sep 15, 2020 · 3 comments
Closed

Remove CUCUMBER_PUBLISH_TOKEN validation #2122

aslakhellesoy opened this issue Sep 15, 2020 · 3 comments
Labels
🐛 bug Defect / Bug

Comments

@aslakhellesoy
Copy link
Contributor

The PublishTokenParser only allows tokens that look like a base64-encoded string. Token validation should only be the server's responsibility. We don't actually want it to be a base64-encoded string, just a UUID.

@mpkorstanje do you remember why we added this?

@aslakhellesoy aslakhellesoy added the 🐛 bug Defect / Bug label Sep 15, 2020
@mpkorstanje
Copy link
Contributor

Garbage in protection. Caching errors early is better in general. Rather then removing the validation you can expand the validation regex to include UUID like values too.

@aslakhellesoy
Copy link
Contributor Author

We don't want to commit to any particular format for authentication tokens. We want to have the flexibility to change this on the server without needing to make a new release of Cucumber-JVM.

The server needs to validate the token anyway, so the only benefit of client validation is early errors for users.

The price of that benefit is too high: Not being able to change the token validation on the server without also having to make a new release of Cucumber-JVM.

@mpkorstanje
Copy link
Contributor

Fair enough. Removing it won't break anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Defect / Bug
Projects
None yet
Development

No branches or pull requests

2 participants