Skip to content

Commit

Permalink
Use more descriptive session mode names
Browse files Browse the repository at this point in the history
Session mode PASSWD is ambiguous, because authentication includes password
authentication. CHANGE_PASSWORD should be clearer.

This also changes session mode AUTH to AUTHENTICATE so that we use a
verb (which describes the purpose of the session) for both session
modes.
  • Loading branch information
adombeck committed Jan 17, 2025
1 parent aba1e3c commit f213b36
Show file tree
Hide file tree
Showing 12 changed files with 111 additions and 110 deletions.
2 changes: 1 addition & 1 deletion examplebroker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ func (b *Broker) NewSession(ctx context.Context, username, lang, mode string) (s
return "", "", fmt.Errorf("user %q does not exist", username)
}

if info.sessionMode == auth.SessionModePasswd {
if info.sessionMode == auth.SessionModeChangePassword {
info.neededAuthSteps++
info.pwdChange = mustReset
}
Expand Down
8 changes: 4 additions & 4 deletions internal/brokers/auth/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ const (
var Replies = []string{Granted, Denied, Cancelled, Retry, Next}

const (
// SessionModeAuth is the name of the authentication session.
SessionModeAuth = "auth"
// SessionModePasswd is the name of the passwd session.
SessionModePasswd = "passwd"
// SessionModeAuthenticate is used when the session is for user authentication.
SessionModeAuthenticate = "auth"
// SessionModeChangePassword is used when the session is for changing the user password.
SessionModeChangePassword = "passwd"
)
2 changes: 1 addition & 1 deletion internal/brokers/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func TestNewSession(t *testing.T) {
wantErr bool
}{
"Successfully start a new auth session": {username: "success"},
"Successfully start a new passwd session": {username: "success", sessionMode: auth.SessionModePasswd},
"Successfully start a new passwd session": {username: "success", sessionMode: auth.SessionModeChangePassword},
"Successfully start a new session with the correct broker": {username: "success", configuredBrokers: []string{t.Name() + "_Broker1.conf", t.Name() + "_Broker2.conf"}},

"Error when broker does not exist": {brokerID: "does_not_exist", wantErr: true},
Expand Down
155 changes: 78 additions & 77 deletions internal/proto/authd/authd.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions internal/proto/authd/authd.proto
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ message StringResponse {

enum SessionMode {
UNDEFINED = 0;
AUTH = 1;
PASSWD = 2;
AUTHENTICATE = 1;
CHANGE_PASSWORD = 2;
}

message SBRequest {
Expand Down
8 changes: 4 additions & 4 deletions internal/services/pam/pam.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,10 @@ func (s Service) SelectBroker(ctx context.Context, req *authd.SBRequest) (resp *

var mode string
switch req.GetMode() {
case authd.SessionMode_AUTH:
mode = auth.SessionModeAuth
case authd.SessionMode_PASSWD:
mode = auth.SessionModePasswd
case authd.SessionMode_AUTHENTICATE:
mode = auth.SessionModeAuthenticate
case authd.SessionMode_CHANGE_PASSWORD:
mode = auth.SessionModeChangePassword
default:
return nil, status.Error(codes.InvalidArgument, "invalid session mode")
}
Expand Down
16 changes: 8 additions & 8 deletions internal/services/pam/pam_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ func TestSelectBroker(t *testing.T) {

wantErr bool
}{
"Successfully select a broker and creates auth session": {username: "success", sessionMode: auth.SessionModeAuth},
"Successfully select a broker and creates passwd session": {username: "success", sessionMode: auth.SessionModePasswd},
"Successfully select a broker and creates auth session": {username: "success", sessionMode: auth.SessionModeAuthenticate},
"Successfully select a broker and creates passwd session": {username: "success", sessionMode: auth.SessionModeChangePassword},

"Error when not root": {username: "success", currentUserNotRoot: true, wantErr: true},
"Error when username is empty": {wantErr: true},
Expand Down Expand Up @@ -229,10 +229,10 @@ func TestSelectBroker(t *testing.T) {

var sessionMode authd.SessionMode
switch tc.sessionMode {
case auth.SessionModeAuth, "":
sessionMode = authd.SessionMode_AUTH
case auth.SessionModePasswd:
sessionMode = authd.SessionMode_PASSWD
case auth.SessionModeAuthenticate, "":
sessionMode = authd.SessionMode_AUTHENTICATE
case auth.SessionModeChangePassword:
sessionMode = authd.SessionMode_CHANGE_PASSWORD
case "-":
sessionMode = authd.SessionMode_UNDEFINED
}
Expand Down Expand Up @@ -571,7 +571,7 @@ func TestIDGeneration(t *testing.T) {
sbResp, err := client.SelectBroker(context.Background(), &authd.SBRequest{
BrokerId: mockBrokerGeneratedID,
Username: usernamePrefix + testutils.IDSeparator + tc.username,
Mode: authd.SessionMode_AUTH,
Mode: authd.SessionMode_AUTHENTICATE,
})
require.NoError(t, err, "Setup: failed to create session for tests")

Expand Down Expand Up @@ -812,7 +812,7 @@ func startSession(t *testing.T, client authd.PAMClient, username string) string
sbResp, err := client.SelectBroker(context.Background(), &authd.SBRequest{
BrokerId: mockBrokerGeneratedID,
Username: username,
Mode: authd.SessionMode_AUTH,
Mode: authd.SessionMode_AUTHENTICATE,
})
require.NoError(t, err, "Setup: failed to create session for tests")
return sbResp.GetSessionId()
Expand Down
4 changes: 2 additions & 2 deletions pam/integration-tests/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func TestCLIAuthenticate(t *testing.T) {

localgroupstestutils.RequireGPasswdOutput(t, gpasswdOutput, golden.Path(t)+".gpasswd_out")

requireRunnerResultForUser(t, authd.SessionMode_AUTH, tc.clientOptions.PamUser, got)
requireRunnerResultForUser(t, authd.SessionMode_AUTHENTICATE, tc.clientOptions.PamUser, got)
})
}
}
Expand Down Expand Up @@ -306,7 +306,7 @@ func TestCLIChangeAuthTok(t *testing.T) {
got := td.ExpectedOutput(t, outDir)
golden.CheckOrUpdate(t, got)

requireRunnerResult(t, authd.SessionMode_PASSWD, got)
requireRunnerResult(t, authd.SessionMode_CHANGE_PASSWORD, got)
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions pam/integration-tests/native_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ func TestNativeAuthenticate(t *testing.T) {
localgroupstestutils.RequireGPasswdOutput(t, gpasswdOutput, golden.Path(t)+".gpasswd_out")

if !tc.skipRunnerCheck {
requireRunnerResultForUser(t, authd.SessionMode_AUTH, tc.clientOptions.PamUser, got)
requireRunnerResultForUser(t, authd.SessionMode_AUTHENTICATE, tc.clientOptions.PamUser, got)
}
})
}
Expand Down Expand Up @@ -435,7 +435,7 @@ func TestNativeChangeAuthTok(t *testing.T) {
golden.CheckOrUpdate(t, got)

if !tc.skipRunnerCheck {
requireRunnerResult(t, authd.SessionMode_PASSWD, got)
requireRunnerResult(t, authd.SessionMode_CHANGE_PASSWORD, got)
}
})
}
Expand Down
Loading

0 comments on commit f213b36

Please sign in to comment.