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

move shib require to top location block #659

Merged
merged 1 commit into from
Feb 14, 2024
Merged

Conversation

umjnega
Copy link
Contributor

@umjnega umjnega commented Feb 13, 2024

Went ahead and looked into the fulcrum issue from yesterday and I think I correclty updated the change @skorner made.

@umjnega umjnega requested review from skorner and antmoth February 13, 2024 15:38
Copy link
Member

@skorner skorner left a comment

Choose a reason for hiding this comment

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

See the comment I added that highlights one more change we have to address for this to work.

Comment on lines -159 to -160
AuthType shibboleth
ShibRequestSetting requireSession 0
Copy link
Member

Choose a reason for hiding this comment

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

These two lines need to be present in the rendered Location / definition (see the diff I posted in slack). You can do this with an auth_require and custom fragment line, as is done here (ignore the auth_require in that example as that is already covered with the require all definition in this manifest). You can probably make this addition to authz_base_requires.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

auth_require or auth_type... I may have pushed my last commit the opposite. But i'll let you correct me if i'm reading this wrong.

Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me! Go ahead and squash to 1 commit and merge after the tests pass from the squash.

add two shib lines

remove whitespace
@umjnega umjnega force-pushed the fulcrum_badrobo_enable branch from 6cb2b4a to d8f9ab5 Compare February 14, 2024 15:03
@umjnega umjnega merged commit 9072e62 into master Feb 14, 2024
1 check passed
@skorner skorner deleted the fulcrum_badrobo_enable branch February 26, 2024 00:37
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.

2 participants