Skip to content
This repository was archived by the owner on Dec 16, 2025. It is now read-only.

feat!: Using jose instead of jws#494

Closed
BjornTheProgrammer wants to merge 2 commits into
googleapis:mainfrom
BjornTheProgrammer:main
Closed

feat!: Using jose instead of jws#494
BjornTheProgrammer wants to merge 2 commits into
googleapis:mainfrom
BjornTheProgrammer:main

Conversation

@BjornTheProgrammer

Copy link
Copy Markdown

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes googleapis/google-cloud-node-core#872 🦕

This pull request would change to using jose, a cross-runtime library for web tokens, instead of jws. The significance of this has been described in the issue. But it would make most if not all googleapis available on platforms like Cloudfare workers.

BREAKING CHANGE:
There are possible breaking changes. The test had to be modified to comply with pkcs8 and 2048 bits long. I could make this support pkcs1 and convert it to pkcs8, but it looks like the actual private keys returned by Google are pkcs8, and this might've been an artifact of just testing. Verification should be done about that before hand. Additionally node support has been bumped to >= 15, as that is when the WebCrypto api's were added to node.

@BjornTheProgrammer BjornTheProgrammer requested a review from a team October 27, 2024 05:48
@conventional-commit-lint-gcf

conventional-commit-lint-gcf Bot commented Oct 27, 2024

Copy link
Copy Markdown

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@product-auto-label product-auto-label Bot added the size: m Pull request size is medium. label Oct 27, 2024
@BjornTheProgrammer BjornTheProgrammer force-pushed the main branch 2 times, most recently from a2fb6b4 to 85be330 Compare October 27, 2024 05:54
Comment thread package.json Outdated
"@types/mocha": "^9.0.0",
"@types/node": "^20.0.0",
"c8": "^9.0.0",
"cheerio": "1.0.0-rc.12",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we can use 1.0.0 now

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Beside that I'm wonder when will this PR got merge... Maybe recent reviewers like @sofisl @d-goog can take a look?

@jonastemplestein

Copy link
Copy Markdown

Hey folks, we just bumped into this. Would be really valuable to the increasing number of cloudflare users to merge this ❤️

1 similar comment
@jonastemplestein

Copy link
Copy Markdown

Hey folks, we just bumped into this. Would be really valuable to the increasing number of cloudflare users to merge this ❤️

@BjornTheProgrammer

Copy link
Copy Markdown
Author

I'm going to rebase this soon, so that it'd be easier to merge if needed.

@igoforth

Copy link
Copy Markdown

I'm going to rebase this soon, so that it'd be easier to merge if needed.

Can't open an issue on the fork so hoping this is the correct place. I've been using your fork for the past few months in a cloudflare worker and it's been great. I started getting the error "Error: unsupported_grant_type: Invalid grant_type:" and suspect the last commit was the cause because it was so recent. It could be something else but just thought I'd let you know. Thanks

@BjornTheProgrammer

Copy link
Copy Markdown
Author

I'm going to rebase this soon, so that it'd be easier to merge if needed.

Can't open an issue on the fork so hoping this is the correct place. I've been using your fork for the past few months in a cloudflare worker and it's been great. I started getting the error "Error: unsupported_grant_type: Invalid grant_type:" and suspect the last commit was the cause because it was so recent. It could be something else but just thought I'd let you know. Thanks

Sorry for potentially breaking something for you. I'll make sure that doesn't happen in the future. For now, make sure that you update the googleapis package that you are using. And let me know if the issue presists. If it does, please make a repo for it so I could test it out.

@igoforth

Copy link
Copy Markdown

I'm going to rebase this soon, so that it'd be easier to merge if needed.

Can't open an issue on the fork so hoping this is the correct place. I've been using your fork for the past few months in a cloudflare worker and it's been great. I started getting the error "Error: unsupported_grant_type: Invalid grant_type:" and suspect the last commit was the cause because it was so recent. It could be something else but just thought I'd let you know. Thanks

Sorry for potentially breaking something for you. I'll make sure that doesn't happen in the future. For now, make sure that you update the googleapis package that you are using. And let me know if the issue presists. If it does, please make a repo for it so I could test it out.

All good, managed to find a workaround using the crypto subtle api.

@sofisl

sofisl commented Jun 10, 2025

Copy link
Copy Markdown
Contributor

Unfortunately I think I will have to wait on considering this change for later this year (not the 8 release). We use this code in auth and currently don't really have good enough integration tests to consider the full ramifications of this type of breaking change. I will, however, make sure to discuss with the team and consider it for the Node 20 change (later this year). Apologies for the delay! Closing for cleanliness but will keep the issue open.

@sofisl sofisl closed this Jun 10, 2025
@BjornTheProgrammer

Copy link
Copy Markdown
Author

No problem, I appreciate the response! In the meantime I'll revert my fork to use the old code so that it works with v7.

@ignoramous

ignoramous commented Jun 13, 2025

Copy link
Copy Markdown

For those coming here from BjornTheProgrammer's blogpost (here), a Cloudflare engineer's stand-alone OAuth2 impl for googleapis works with Workers: https://github.com/Schachte/cloudflare-google-auth

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants