Skip to content

Fix SAMLForceAuthn marshalling#48098

Merged
Joerger merged 2 commits intomasterfrom
joerger/fix-sso-mfa-settings-marshalling
Oct 30, 2024
Merged

Fix SAMLForceAuthn marshalling#48098
Joerger merged 2 commits intomasterfrom
joerger/fix-sso-mfa-settings-marshalling

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Oct 29, 2024

Follow up to #46703 to fix json/yaml marshalling to/from string/bool. This way, you can set it like:

kind: saml
version: v2
metadata:
  name: okta
spec:
  ...
  force_authn: yes
  ...

@Joerger Joerger added backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry labels Oct 29, 2024
@github-actions github-actions Bot requested review from bl-nero and kimlisa October 29, 2024 19:20
Comment thread api/types/mfa.go Outdated
}

// MarshalJSON marshals SAMLForceAuthn to string.
func (s *SAMLForceAuthn) MarshalYAML() (interface{}, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here and everywhere else prefer using any over interface{}

Suggested change
func (s *SAMLForceAuthn) MarshalYAML() (interface{}, error) {
func (s *SAMLForceAuthn) MarshalYAML() (any, error) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This actually need to be interface{} for this to implement yaml.Marshaler, apparently interface{} isn't treated exactly like any. Changed it where I could though.

Comment thread api/types/mfa.go Outdated
Comment thread api/types/mfa.go Outdated
Comment thread api/types/mfa.go Outdated
Comment thread api/types/mfa.go Outdated
Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy left a comment

Choose a reason for hiding this comment

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

Can you add some test coverage?

@Joerger Joerger force-pushed the joerger/fix-sso-mfa-settings-marshalling branch from 2d272f0 to fc9990f Compare October 30, 2024 17:20
@Joerger Joerger force-pushed the joerger/fix-sso-mfa-settings-marshalling branch from fc9990f to e3b49d2 Compare October 30, 2024 18:11
@Joerger Joerger enabled auto-merge October 30, 2024 18:15
@Joerger Joerger added this pull request to the merge queue Oct 30, 2024
Merged via the queue into master with commit 3d3ee99 Oct 30, 2024
@Joerger Joerger deleted the joerger/fix-sso-mfa-settings-marshalling branch October 30, 2024 18:52
@public-teleport-github-review-bot
Copy link
Copy Markdown

@Joerger See the table below for backport results.

Branch Result
branch/v17 Create PR

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

Labels

backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry size/md

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants