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

Make preauthentication based on headers optional #441

Closed
wants to merge 1 commit into from

Conversation

nbarrientos
Copy link
Contributor

@nbarrientos nbarrientos commented Feb 17, 2020

Pull Request (PR) description

Users might only want to configure preauthentication using request attributes, without making use of any header. The issue is that in that scenario setting the header-related settings to empty values seems to break things. I think it's probably a good idea to set them only if the relevant keys are present in preauthenticated_config.

@nbarrientos nbarrientos changed the title WIP Make preauth based on headers optional Make preauthentication based on headers optional Feb 19, 2020
@nbarrientos
Copy link
Contributor Author

Ready to be reviewed.

@nbarrientos
Copy link
Contributor Author

Similar to #439. I think it's fine to keep the redirectUrl setting as is.

@bastelfreak bastelfreak added enhancement New feature or request needs-tests labels May 9, 2020
@bastelfreak
Copy link
Member

thanks for the work @nbarrientos . can you please add tests that verify if the filecontent is present/absent?

@nbarrientos nbarrientos force-pushed the ibarrien_preauth branch 2 times, most recently from 47b8ce4 to f29696c Compare May 12, 2020 07:43
@nbarrientos
Copy link
Contributor Author

Hi @bastelfreak

Thanks for your comments. I've added a test context to make sure that the corresponding lines are not included if the keys are not present in the config hash. The opposite case is already covered by the tests as both keys are set in the default configuration (see rundeck::config::global::rundeck_config class without any parameters on).

Btw, I'd suggest to close #439.

@vox-pupuli-tasks
Copy link

Dear @nbarrientos, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@Joris29
Copy link
Contributor

Joris29 commented Dec 7, 2023

@kenyon This is fixed by #523

@kenyon kenyon closed this Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants