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

Ensure other authentication options can be used #34

Merged
merged 14 commits into from
Jul 1, 2024

Conversation

LDSamson
Copy link
Collaborator

Addresses #33.

Creates different configuration options with and without shinymanager, so that other login options can be used. In case other login methods will be used, the user information will either be obtained directly from the shiny session (e.g. Posit connect deployment), or from the HTTP headers within the session (ShinyProxy deployment).

@LDSamson LDSamson requested a review from jthompson-arcus June 20, 2024 12:43
Copy link
Collaborator

@jthompson-arcus jthompson-arcus left a comment

Choose a reason for hiding this comment

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

Some comments and discussion starters...

Using golem::app_prod() in place of test_mode

We ran into issues trying to do this on a separate project. It mostly boiled down to the fact that how golem has this setup, a user has to opt-in to run the application in production mode (i.e. that have to set the option golem.app.prod to TRUE).

Configuration file

Perhaps we should consider using a separate configuration file for testing purposes. I think it's a little confusing to have both a test_shinymanager and a prod_shinymanager. Plus you have already set it up so that the file path can be set using the environmental variable CONFIG_PATH. Maybe we should make a test-config.yml.

R/app_authentication.R Outdated Show resolved Hide resolved
R/app_authentication.R Outdated Show resolved Hide resolved
Comment on lines 46 to 48
user_id: user
user_name: user
user_group: groups
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it make sense to allow these to be set in deployment when user_identification determines their value. Particularly in a case like shiny_session.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do not completely follow here. To be set how exactly? Not in the golem-config.yml file, but somewhere else?

inst/golem-config.yml Show resolved Hide resolved
@LDSamson
Copy link
Collaborator Author

I agree that both a test_shinymanager and prod_shinymanager config might be confusing. The biggest difference is whether app_prod is set to 'yes'. We can just make one shinymanager config, with app_prod set to yes, and then temporarily set options(golem.app.prod = FALSE) in any test where that is needed.

Regarding the use of golem::app_prod() Would this problem still occur if users set their specific configuration, and we set the option app_prod in these? Since this will set the variable golem::app_prod() as well?

@jthompson-arcus
Copy link
Collaborator

I agree that both a test_shinymanager and prod_shinymanager config might be confusing. The biggest difference is whether app_prod is set to 'yes'. We can just make one shinymanager config, with app_prod set to yes, and then temporarily set options(golem.app.prod = FALSE) in any test where that is needed.

Regarding the use of golem::app_prod() Would this problem still occur if users set their specific configuration, and we set the option app_prod in these? Since this will set the variable golem::app_prod() as well?

The result of golem::app_prod() is not actually being set in the configuration file.

> golem::app_prod
function () 
{
    getOption("golem.app.prod") %||% FALSE
}
<bytecode: 0x0000021537725a20>
<environment: namespace:golem>

@LDSamson
Copy link
Collaborator Author

I agree that both a test_shinymanager and prod_shinymanager config might be confusing. The biggest difference is whether app_prod is set to 'yes'. We can just make one shinymanager config, with app_prod set to yes, and then temporarily set options(golem.app.prod = FALSE) in any test where that is needed.
Regarding the use of golem::app_prod() Would this problem still occur if users set their specific configuration, and we set the option app_prod in these? Since this will set the variable golem::app_prod() as well?

The result of golem::app_prod() is not actually being set in the configuration file.

> golem::app_prod
function () 
{
    getOption("golem.app.prod") %||% FALSE
}
<bytecode: 0x0000021537725a20>
<environment: namespace:golem>

Ah my bad, that is confusing!

@LDSamson LDSamson linked an issue Jul 1, 2024 that may be closed by this pull request
@LDSamson LDSamson merged commit 7f5b81c into dev Jul 1, 2024
2 checks passed
@LDSamson LDSamson deleted the 033_alternative_login_options branch July 8, 2024 10:35
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.

Allow user management and login options other than shinymanager
2 participants