-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Support for additional claims in the JWT tokens, needed for Google's Identity-Aware Proxy #570
Conversation
This is the google docs for it: |
@hadley any comments? Should I change something? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a bullet to the top of NEWS.md
? It should briefly describe the change and end with (@yourname, #issuenumber)
.
DESCRIPTION
Outdated
@@ -1,6 +1,6 @@ | |||
Package: httr | |||
Title: Tools for Working with URLs and HTTP | |||
Version: 1.4.0.9000 | |||
Version: 1.4.1.9000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't change the version number
@@ -42,6 +43,7 @@ init_oauth_service_account <- function(secrets, scope = NULL, sub = NULL) { | |||
#' 00:00:00 UTC, January 1, 1970. This value has a maximum of 1 hour from | |||
#' the issued time. | |||
#' @param duration Duration of token, in seconds. | |||
#' @param ... any additional claims for the token. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#' @param ... any additional claims for the token. | |
#' @param ... Any additional claims for the token. |
@@ -301,6 +301,7 @@ Token2.0 <- R6::R6Class("Token2.0", inherit = Token, list( | |||
#' @inheritParams oauth2.0_token | |||
#' @inheritParams jwt_signature | |||
#' @param secrets Secrets loaded from JSON file, downloaded from console. | |||
#' @param ... any additional claims for the token |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove this line since we're already inheriting docs from jwt_signature
?
@@ -311,7 +312,7 @@ Token2.0 <- R6::R6Class("Token2.0", inherit = Token, list( | |||
#' | |||
#' token <- oauth_service_token(endpoint, secrets, scope) | |||
#' } | |||
oauth_service_token <- function(endpoint, secrets, scope = NULL, sub = NULL) { | |||
oauth_service_token <- function(endpoint, secrets, scope = NULL, sub = NULL, ...) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using ...
here seems a bit broad. I'd think it should be claims
or similar?
httr has been superseded in favour of httr2, so is no longer under active development. Thanks for using httr, thanks for contributing, and my apologies that your contribution never made it into the package. |
Hi and thanks for an excellent package!
We're using Google Cloud's Identity-Aware Proxy (IAP) for which
httr
is almost compatible.The only discrepancy is that IAP requires an additional "claim" on the JWT.
This pull requests:
oauth_service_token
id_token
ifaccess_token
is not set (which is the case for IAP tokens)Using this for IAP is now very simple: