Skip to content

Fix panic on nil value in getPresetRoles()#35458

Merged
zmb3 merged 3 commits intomasterfrom
timothyb89/fix-nil-preset-roles
Dec 7, 2023
Merged

Fix panic on nil value in getPresetRoles()#35458
zmb3 merged 3 commits intomasterfrom
timothyb89/fix-nil-preset-roles

Conversation

@timothyb89
Copy link
Copy Markdown
Contributor

This fixes a panic when attempting to fetch /webapi/presetroles as ui.NewRoles() is (reasonably) not nil safe. GetPresetRoles() can return nil entries if corresponding features are disabled, so this tweaks GetPresetRoles() to filter out nil entries.

This endpoint is only used for Cloud features (email invites at onboarding, specifically) and currently all features are enabled for Cloud users, so the only internal use is minimally impacted.

The other use of GetPresetRoles(), createPresetRoles(), already checks for nil entries. It's behavior is unchanged.

changelog: Fix panic on potential nil value when requesting /webapi/presetroles

This fixes a panic when attempting to fetch `/webapi/presetroles` as
`ui.NewRoles()` is (reasonably) not nil safe. `GetPresetRoles()` can
return nil entries if corresponding features are disabled, so this
tweaks `GetPresetRoles()` to filter out nil entries.

This endpoint is only used for Cloud features (email invites at
onboarding, specifically) and currently all features are enabled for
Cloud users, so the only internal use is minimally impacted.

The other use of `GetPresetRoles()`, `createPresetRoles()`, already
checks for nil entries. It's behavior is unchanged.
@timothyb89 timothyb89 requested a review from zmb3 December 7, 2023 01:33
Comment thread lib/auth/init.go Outdated
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from hugoShaka December 7, 2023 01:43
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
@timothyb89 timothyb89 enabled auto-merge December 7, 2023 01:46
@zmb3 zmb3 disabled auto-merge December 7, 2023 01:47
@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Dec 7, 2023

Disabled auto merge in case you want to accept my suggestion.

Edit: whoops, you already did! 😅

@zmb3 zmb3 enabled auto-merge December 7, 2023 01:47
@timothyb89
Copy link
Copy Markdown
Contributor Author

Disabled auto merge in case you want to accept my suggestion.

Edit: whoops, you already did! 😅

Yup, sorry, it just needed a tiny build fix. Sorry for the confusion!

@zmb3 zmb3 added this pull request to the merge queue Dec 7, 2023
Merged via the queue into master with commit b92d40c Dec 7, 2023
@zmb3 zmb3 deleted the timothyb89/fix-nil-preset-roles branch December 7, 2023 02:22
@public-teleport-github-review-bot
Copy link
Copy Markdown

@timothyb89 See the table below for backport results.

Branch Result
branch/v13 Create PR
branch/v14 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants