Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions api/types/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ package types
import (
"net/netip"
"net/url"
"os"
"slices"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -575,8 +577,15 @@ func (o *OIDCConnectorV3) WithMFASettings() error {
o.Spec.ClientSecret = o.Spec.MFASettings.ClientSecret
o.Spec.ACR = o.Spec.MFASettings.AcrValues
o.Spec.Prompt = o.Spec.MFASettings.Prompt
o.Spec.MaxAge = &MaxAge{
Value: o.Spec.MFASettings.MaxAge,
// In rare cases, some providers will complain about the presence of the 'max_age'
// parameter in auth requests. Provide users with a workaround to omit it.
omitMaxAge, _ := strconv.ParseBool(os.Getenv("TELEPORT_OIDC_OMIT_MFA_MAX_AGE"))
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.

Just to make sure we don't lose that workaround, can you make sure we document the correct way to use this?

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.

I believe that our intention is to avoid advertising this option in the docs. While some users really do need this option, it's a potentially significant security footgun for most.

I can (and probably should) add a corresponding test case to the OIDC implementation that will squawk if this behavior is accidentally removed or broken though.

if omitMaxAge {
o.Spec.MaxAge = nil
} else {
o.Spec.MaxAge = &MaxAge{
Value: o.Spec.MFASettings.MaxAge,
}
}
return nil
}
Expand Down
Loading