Skip to content

Use ssh_service.public_addrs in IsMFARequired check #24070

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.

Sign up for GitHub

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

Merged
Joerger merged 1 commit into from
Apr 26, 2023
Merged

Use ssh_service.public_addrs in IsMFARequired check #24070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions api/proto/teleport/legacy/types/types.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -704,9 +704,10 @@ message ServerSpecV2 {
reserved 8;
reserved 10;

// Addr is server host:port address
// Addr is a host:port address where this server can be reached.
string Addr = 1 [(gogoproto.jsontag) = "addr"];
// PublicAddr is the public address this cluster can be reached at.
// PublicAddr is the public address where this server can be reached.
// DELETE IN 15.0. (joerger) Deprecated in favor of public_addrs.
string PublicAddr = 2 [(gogoproto.jsontag) = "public_addr,omitempty"];
// Hostname is server hostname
string Hostname = 3 [(gogoproto.jsontag) = "hostname"];
Expand Down Expand Up @@ -736,6 +737,8 @@ message ServerSpecV2 {
string PeerAddr = 11 [(gogoproto.jsontag) = "peer_addr,omitempty"];
// ProxyIDs is a list of proxy IDs this server is expected to be connected to.
repeated string ProxyIDs = 12 [(gogoproto.jsontag) = "proxy_ids,omitempty"];
// PublicAddrs is a list of public addresses where this server can be reached.
repeated string public_addrs = 13;
}

// AppServerV3 represents a single proxied web app.
Expand Down
40 changes: 30 additions & 10 deletions api/types/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ type Server interface {
GetCmdLabels() map[string]CommandLabel
// SetCmdLabels sets command labels.
SetCmdLabels(cmdLabels map[string]CommandLabel)
// GetPublicAddr is an optional field that returns the public address this cluster can be reached at.
// GetPublicAddr returns a public address where this server can be reached.
GetPublicAddr() string
// GetPublicAddrs returns a list of public addresses where this server can be reached.
GetPublicAddrs() []string
// GetRotation gets the state of certificate authority rotation.
GetRotation() Rotation
// SetRotation sets the state of certificate authority rotation.
Expand All @@ -63,8 +65,8 @@ type Server interface {
String() string
// SetAddr sets server address
SetAddr(addr string)
// SetPublicAddr sets the public address this cluster can be reached at.
SetPublicAddr(string)
// SetPublicAddrs sets the public addresses where this server can be reached.
SetPublicAddrs([]string)
// SetNamespace sets server namespace
SetNamespace(namespace string)
// GetApps gets the list of applications this server is proxying.
Expand Down Expand Up @@ -178,9 +180,13 @@ func (s *ServerV2) Expiry() time.Time {
return s.Metadata.Expiry()
}

// SetPublicAddr sets the public address this cluster can be reached at.
func (s *ServerV2) SetPublicAddr(addr string) {
s.Spec.PublicAddr = addr
// SetPublicAddrs sets the public proxy addresses where this server can be reached.
func (s *ServerV2) SetPublicAddrs(addrs []string) {
s.Spec.PublicAddrs = addrs
Comment thread
Joerger marked this conversation as resolved.
Outdated
// DELETE IN 15.0. (Joerger) PublicAddr deprecated in favor of PublicAddrs
if len(addrs) != 0 {
s.Spec.PublicAddr = addrs[0]
}
}

// GetName returns server name
Expand All @@ -198,9 +204,22 @@ func (s *ServerV2) GetAddr() string {
return s.Spec.Addr
}

// GetPublicAddr is an optional field that returns the public address this cluster can be reached at.
// GetPublicAddr returns a public address where this server can be reached.
func (s *ServerV2) GetPublicAddr() string {
Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy Apr 18, 2023

Choose a reason for hiding this comment

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

Is this still needed? Should all callers be using GetPublicAddrs which will fallback to just returning the PublicAddr if PublicAddrs are not set?

Copy link
Copy Markdown
Contributor Author

@Joerger Joerger Apr 24, 2023

Choose a reason for hiding this comment

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

Now this is just a helper function to return the first addr or empty string so we don't need to do the following for every caller:

var addr string
if addrs := s.GetPublicAddrs(); len(addrs) != 0 {
	adrr = s.Spec.PublicAddrs[0]
}

return s.Spec.PublicAddr
addrs := s.GetPublicAddrs()
if len(addrs) != 0 {
return addrs[0]
}
return ""
}

// GetPublicAddrs returns a list of public addresses where this server can be reached.
func (s *ServerV2) GetPublicAddrs() []string {
// DELETE IN 15.0. (Joerger) PublicAddr deprecated in favor of PublicAddrs
if len(s.Spec.PublicAddrs) == 0 && s.Spec.PublicAddr != "" {
return []string{s.Spec.PublicAddr}
}
return s.Spec.PublicAddrs
}

// GetRotation gets the state of certificate authority rotation.
Expand Down Expand Up @@ -404,8 +423,8 @@ func (s *ServerV2) CheckAndSetDefaults() error {
if s.Spec.Addr == "" {
return trace.BadParameter(`Addr must be set when server SubKind is "openssh"`)
}
if s.Spec.PublicAddr != "" {
return trace.BadParameter(`PublicAddr must not be set when server SubKind is "openssh"`)
if len(s.GetPublicAddrs()) != 0 {
return trace.BadParameter(`PublicAddrs must not be set when server SubKind is "openssh"`)
}
if s.Spec.Hostname == "" {
return trace.BadParameter(`Hostname must be set when server SubKind is "openssh"`)
Expand Down Expand Up @@ -436,6 +455,7 @@ func (s *ServerV2) MatchSearch(values []string) bool {

if s.GetKind() == KindNode {
fieldVals = append(utils.MapToStrings(s.GetAllLabels()), s.GetName(), s.GetHostname(), s.GetAddr())
fieldVals = append(fieldVals, s.GetPublicAddrs()...)

if s.GetUseTunnel() {
custom = func(val string) bool {
Expand Down
32 changes: 16 additions & 16 deletions api/types/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ func TestServerCheckAndSetDefaults(t *testing.T) {
Namespace: defaults.Namespace,
},
Spec: ServerSpecV2{
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddr: "1.2.3.4:3080",
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddrs: []string{"1.2.3.4:3080"},
},
},
assertion: func(t *testing.T, s *ServerV2, err error) {
Expand All @@ -137,9 +137,9 @@ func TestServerCheckAndSetDefaults(t *testing.T) {
Namespace: defaults.Namespace,
},
Spec: ServerSpecV2{
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddr: "1.2.3.4:3080",
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddrs: []string{"1.2.3.4:3080"},
},
}
require.Equal(t, expectedServer, s)
Expand All @@ -155,9 +155,9 @@ func TestServerCheckAndSetDefaults(t *testing.T) {
Namespace: defaults.Namespace,
},
Spec: ServerSpecV2{
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddr: "1.2.3.4:3080",
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddrs: []string{"1.2.3.4:3080"},
},
},
assertion: func(t *testing.T, s *ServerV2, err error) {
Expand All @@ -170,9 +170,9 @@ func TestServerCheckAndSetDefaults(t *testing.T) {
Namespace: defaults.Namespace,
},
Spec: ServerSpecV2{
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddr: "1.2.3.4:3080",
Addr: "1.2.3.4:3022",
Hostname: "teleport-node",
PublicAddrs: []string{"1.2.3.4:3080"},
},
}
require.Equal(t, expectedServer, s)
Expand Down Expand Up @@ -274,13 +274,13 @@ func TestServerCheckAndSetDefaults(t *testing.T) {
Namespace: defaults.Namespace,
},
Spec: ServerSpecV2{
Addr: "1.2.3.4:3022",
Hostname: "openssh-node",
PublicAddr: "1.2.3.4:80",
Addr: "1.2.3.4:3022",
Hostname: "openssh-node",
PublicAddrs: []string{"1.2.3.4:80"},
},
},
assertion: func(t *testing.T, s *ServerV2, err error) {
require.EqualError(t, err, `PublicAddr must not be set when server SubKind is "openssh"`)
require.EqualError(t, err, `PublicAddrs must not be set when server SubKind is "openssh"`)
},
},
{
Expand Down
Loading