Skip to content

Use base64 encoding for usernames in http headers (shinyproxy specific) #123

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

Merged
merged 3 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Package: clinsight
Title: ClinSight
Version: 0.1.0.9004
Version: 0.1.0.9006
Authors@R: c(
person("Leonard Daniël", "Samson", , "[email protected]", role = c("cre", "aut"),
comment = c(ORCID = "0000-0002-6252-7639")),
Expand Down Expand Up @@ -46,6 +46,7 @@ Imports:
tools,
withr
Suggests:
base64enc,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have mixed feelings about this but since we are including base64enc in the suggests, I think decode_base64() should at the very list check if the packaged has been installed. (I have mixed feelings because it is a dependency for other packages, so I can't really think of a scenario where it wouldn't be available.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is a function that will only run in production. It should be installed already since the package bslib depends on it. What other reservations do you have for adding it in suggests, other than checking in decode_base64 a check if the package is installed?

I will add the check mentioned by you in the function.

kableExtra,
knitr,
pkgload,
Expand Down
6 changes: 6 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
# clinsight (development version)

## Changed

- Added `pkgdown` GHA workflow to automatically update documentation site with PRs & pushes to `main` and `dev`
- Generalized `merge_meta_with_data()` to allow user-defined processing functions.
- Added a feature where, in applicable tables, a user can navigate to a form by double-clicking a table row.
- Fixed warnings in `apply_edc_specific_changes` due to the use of a vector within `dplyr::select`.

## Bug fixes

- When using the `shinyproxy` deployment configuration, the user name is now expected to be base64 encoded, and will now be base64 encoded by `clinsight` by default, so that the app can also handle non-ASCII signs in user names that are stored in HTTP headers. To display the user name correctly, use base64 encoding in the `application.yml` in ShinyProxy settings (for example: `http-headers.X_SP_USERNAME: "#{T(java.util.Base64).getEncoder().encodeToString(oidcUser.getFullName().getBytes())}"`).

# clinsight 0.1.0

## Changed
Expand Down
2 changes: 1 addition & 1 deletion R/app_authentication.R
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ authenticate_server <- function(
),
http_headers = reactiveValues(
user = session$request$HTTP_X_SP_USERID,
name = session$request$HTTP_X_SP_USERNAME,
name = decode_base64(session$request$HTTP_X_SP_USERNAME),
roles = get_valid_roles(session$request$HTTP_X_SP_USERGROUPS, all_roles),
sites = all_sites
),
Expand Down
39 changes: 39 additions & 0 deletions R/fct_utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -717,3 +717,42 @@ dblclick_to_form <- function(bttn_ns) {
"document.getElementById(", deparse(NS(bttn_ns, "go_to_form")), ").click();",
"})"
)}

#' Decode a base64 encoded string.
#'
#' Used to decode base64-encoded HTTP headers. Encoding these headers
#' ensures that they can contain complex names with special characters.
#'
#' The function will warn if the decoded output string is no valid UTF-8. This
#' might occur if the input was not base64 encoded.
#'
#' @param x A base64 encoded character string.
#'
#' @return A decoded character string.
#' @noRd
#'
#' @examples
#' encoded_username <- base64enc::base64encode(charToRaw("Ťěšťůšěř 的"))
#' decode_base64(encoded_username)
#'
decode_base64 <- function(
x
){
stopifnot("base64enc is not installed" = rlang::is_installed("base64enc"))
if(is.null(x)) return(x)
stopifnot(is.character(x))
decoded <- base64enc::base64decode(x)
valid_UTF8 <- base64enc::checkUTF8(decoded, quiet = TRUE)

if(isFALSE(valid_UTF8)){
warning(
"decoded string does not contain valid UTF-8. ",
"Is the input really base64 encoded?"
)
# using deparse creates visible quotes around the string but doing so will
# prevent the app from breaking due to incorrect UTF-8 encoding:
return({deparse1(rawToChar(decoded))})
}
rawToChar(decoded)
}

2 changes: 1 addition & 1 deletion inst/golem-config.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
default:
golem_name: clinsight
golem_version: 0.1.0.9005
golem_version: 0.1.0.9006
app_prod: no
user_identification: test_user
study_data: !expr clinsight::clinsightful_data
Expand Down
25 changes: 25 additions & 0 deletions tests/testthat/test-fct_utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -617,3 +617,28 @@ describe("expectation_type() works", {
expect_error(expectation_type(expectation("failure", "f"), "non-existing expectation"))
})
})

describe("decode_base64() works", {
it("converts a base64-encoded string with special characters as expected", {
testthat::skip_if_not_installed("base64enc")
#base64enc::base64encode(charToRaw("Ťěšťůšěř 的"))
expect_equal(
decode_base64("xaTEm8WhxaXFr8WhxJvFmSDnmoQ="),
"Ťěšťůšěř 的"
)
})
it("returns NULL if the input is NULL", {
testthat::skip_if_not_installed("base64enc")
expect_null(decode_base64(NULL))
})
it("warns if decoded output is not a UTF-8 encoded string and
returns a safe, quoted string in such a case", {
testthat::skip_if_not_installed("base64enc")
expect_warning(
output <- decode_base64("I am not encoded"),
paste0("decoded string does not contain valid UTF-8. ",
"Is the input really base64 encoded?")
)
expect_equal(output, "\"!\\xa9\\xa7\\xa2קr\\x87^\"")
})
})
Loading