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

Allow port forwarding to be disabled. #3208

Merged
merged 1 commit into from
Dec 13, 2019
Merged

Allow port forwarding to be disabled. #3208

merged 1 commit into from
Dec 13, 2019

Conversation

russjones
Copy link
Contributor

Description

If the option for port forwarding is not specified, it's enabled by default. Port forwarding is not specified in the default-implicit-role. Since it's included in all role sets, port forwarding is always enabled for all roles.

To fix this, port forwarding in the default-implicit-role is set to false.

If the option for port forwarding is not specified, it's enabled by
default. Port forwarding is not specified in the default-implicit-role.
Since it's included in all role sets, port forwarding is always
enabled for all roles.

To fix this, port forwarding in the default-implicit-role is set to
false.
Copy link
Contributor

@fspmarshall fspmarshall left a comment

Choose a reason for hiding this comment

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

Changes look good; defaulting to more limited privilege is great.

Does this impact current users? Might be problematic if this changes behavior of current configurations which leave the field unspecified.

@russjones
Copy link
Contributor Author

@fspmarshall Unfortunately this change won't limit default behavior. That's what the previous #3207 PR did, but I backed it out in-favor of this PR for the exact reason you said: it would break Teleport for people that had left port forwarding unspecified in their role.

@russjones russjones merged commit 17f94f5 into master Dec 13, 2019
@russjones russjones deleted the rjones/fix-rbac branch December 13, 2019 19:16
This was referenced Aug 6, 2024
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