Skip to content
Open
Show file tree
Hide file tree
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
7 changes: 7 additions & 0 deletions authd-oidc-brokers/internal/broker/authresponses.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,20 @@ package broker

import "github.com/canonical/authd/authd-oidc-brokers/internal/providers/info"

// cachedPasswordMessage is the user-facing notice attached to a granted
// response after the user's Entra password is saved as the local password
// (during the Entra password + MFA flow). It is broker-owned so it can be
// localized independently of authd.
const cachedPasswordMessage = "Your local password has been set to your Entra password"

type isAuthenticatedDataResponse interface {
isAuthenticatedDataResponse()
}

// userInfoMessage represents the user information message that is returned to authd.
type userInfoMessage struct {
UserInfo info.User `json:"userinfo"`
Message string `json:"message,omitempty"`
Comment thread
adombeck marked this conversation as resolved.
}

func (userInfoMessage) isAuthenticatedDataResponse() {}
Expand Down
5 changes: 5 additions & 0 deletions authd-oidc-brokers/internal/broker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -1476,6 +1476,11 @@ func (b *Broker) finishEntraAuth(ctx context.Context, session *session, mfaToken
return AuthDenied, unexpectedErrMsg("failed to store password")
}
session.entraPasswordHash = ""

if msg, ok := data.(userInfoMessage); ok {
msg.Message = cachedPasswordMessage
data = msg
}
}

return access, data
Expand Down
7 changes: 7 additions & 0 deletions authd-oidc-brokers/internal/broker/broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1917,6 +1917,13 @@ func TestIsAuthenticatedEntraMFAWaitStartsPollingAtOne(t *testing.T) {
require.Equal(t, []int{1}, provider.recordedPollAttempts)
require.Equal(t, []string{""}, provider.recordedChallengeData)

var grantPayload struct {
Message string `json:"message"`
}
require.NoError(t, json.Unmarshal([]byte(data), &grantPayload))
require.Equal(t, broker.CachedPasswordMessage, grantPayload.Message,
"Entra MFA completion should attach the offline-password caching notice")

_, err = os.Stat(b.PasswordFilepathForSession(sessionID))
require.NoError(t, err, "Entra MFA completion should cache the offline password")
_, err = os.Stat(b.TokenPathForSession(sessionID))
Expand Down
3 changes: 3 additions & 0 deletions authd-oidc-brokers/internal/broker/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,6 @@ const MaxRequestDuration = maxRequestDuration

// MaxAuthAttempts exposes the broker's maxAuthAttempts for tests.
const MaxAuthAttempts = maxAuthAttempts

// CachedPasswordMessage exposes the broker's cachedPasswordMessage for tests.
const CachedPasswordMessage = cachedPasswordMessage
50 changes: 37 additions & 13 deletions internal/brokers/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ import (
// LocalBrokerName is the name of the local broker.
const LocalBrokerName = "local"

// grantedData is the canonical envelope used to carry a granted authentication
// result between the broker layer and the PAM service. The optional message is
// an authd-controlled, user-facing notice (for example, a caching indicator)
// that the broker may attach on a successful login.
type grantedData struct {
UserInfo types.UserInfo `json:"userinfo"`
Message string `json:"message,omitempty"`
}

type brokerer interface {
NewSession(ctx context.Context, username, lang, mode string) (sessionID, encryptionKey string, err error)
GetAuthenticationModes(ctx context.Context, sessionID string, supportedUILayouts []map[string]string) (authenticationModes []map[string]string, err error)
Expand Down Expand Up @@ -207,9 +216,14 @@ func (b Broker) IsAuthenticated(ctx context.Context, sessionID, authenticationDa

switch access {
case auth.Granted:
rawUserInfo, err := unmarshalAndGetKey(data, "userinfo")
if err != nil {
return "", "", err
var rawData map[string]json.RawMessage
if err := json.Unmarshal([]byte(data), &rawData); err != nil {
return "", "", fmt.Errorf("response returned by the broker is not a valid json: %v\nBroker returned: %v", err, data)
}

rawUserInfo, ok := rawData["userinfo"]
if !ok {
return "", "", fmt.Errorf("missing key %q in returned message, got: %v", "userinfo", data)
}

info, err := unmarshalUserInfo(rawUserInfo)
Expand All @@ -221,22 +235,33 @@ func (b Broker) IsAuthenticated(ctx context.Context, sessionID, authenticationDa
return "", "", err
}

d, err := json.Marshal(info)
var message string
if rawMessage := rawData["message"]; rawMessage != nil {
if err := json.Unmarshal(rawMessage, &message); err != nil {
// A non-string message must not fail an already-granted login; it's cosmetic.
log.Warningf(ctx, "Ignoring non-string message in broker granted response: %v", err)
}
}

// Always forward a consistent {"userinfo": ..., "message": ...} envelope
// (message omitted when empty) so the consumer always parses the same
// shape regardless of whether the broker attached a success message.
d, err := json.Marshal(grantedData{UserInfo: info, Message: message})
Comment thread
adombeck marked this conversation as resolved.
if err != nil {
return "", "", fmt.Errorf("can't marshal UserInfo: %v", err)
}
data = string(d)

case auth.Denied, auth.Retry:
if _, err := unmarshalAndGetKey(data, "message"); err != nil {
if err := requireKey(data, "message"); err != nil {
return "", "", err
}

case auth.Next:
if data == "{}" {
break
}
if _, err := unmarshalAndGetKey(data, "message"); err != nil {
if err := requireKey(data, "message"); err != nil {
return "", "", err
}

Expand Down Expand Up @@ -411,17 +436,16 @@ func validateUserInfo(uInfo types.UserInfo) (err error) {
return nil
}

// unmarshalAndGetKey tries to unmarshal the content in data and returns the value of the requested key.
func unmarshalAndGetKey(data, key string) (json.RawMessage, error) {
// requireKey unmarshals data and returns an error if the given key is missing.
func requireKey(data, key string) error {
var returnedData map[string]json.RawMessage
if err := json.Unmarshal([]byte(data), &returnedData); err != nil {
return nil, fmt.Errorf("response returned by the broker is not a valid json: %v\nBroker returned: %v", err, data)
return fmt.Errorf("response returned by the broker is not a valid json: %v\nBroker returned: %v", err, data)
}

rawMsg, ok := returnedData[key]
if !ok {
return nil, fmt.Errorf("missing key %q in returned message, got: %v", key, data)
if _, ok := returnedData[key]; !ok {
return fmt.Errorf("missing key %q in returned message, got: %v", key, data)
}

return rawMsg, nil
return nil
}
2 changes: 2 additions & 0 deletions internal/brokers/broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,8 @@ func TestIsAuthenticated(t *testing.T) {
cancelFirstCall bool
}{
"Successfully_authenticate": {sessionID: "success"},
"Successfully_authenticate_with_granted_message": {sessionID: "ia_granted_with_data"},
"Ignores_non_string_message_in_granted_response": {sessionID: "ia_granted_with_non_string_message"},
"Successfully_authenticate_after_cancelling_first_call": {sessionID: "ia_second_call", secondCall: true},
"Denies_authentication_when_broker_times_out": {sessionID: "ia_timeout"},
"Adds_default_groups_even_if_broker_did_not_set_them": {sessionID: "ia_info_empty_groups"},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FIRST CALL:
access: granted
data: {"Name":"ia_info_empty_groups@example.com","UID":0,"Gecos":"gecos for ia_info_empty_groups@example.com","Dir":"/home/ia_info_empty_groups@example.com","Shell":"/bin/sh/ia_info_empty_groups@example.com","Groups":[]}
data: {"userinfo":{"Name":"ia_info_empty_groups@example.com","UID":0,"Gecos":"gecos for ia_info_empty_groups@example.com","Dir":"/home/ia_info_empty_groups@example.com","Shell":"/bin/sh/ia_info_empty_groups@example.com","Groups":[]}}
err: <nil>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
FIRST CALL:
access: granted
data: {"userinfo":{"Name":"ia_granted_with_non_string_message@example.com","UID":0,"Gecos":"gecos for ia_granted_with_non_string_message@example.com","Dir":"/home/ia_granted_with_non_string_message@example.com","Shell":"/bin/sh/ia_granted_with_non_string_message@example.com","Groups":[{"Name":"group-ia_granted_with_non_string_message@example.com","GID":null,"UGID":"ugid-ia_granted_with_non_string_message@example.com"}]}}
err: <nil>
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FIRST CALL:
access: granted
data: {"Name":"ia_info_empty_gecos@example.com","UID":0,"Gecos":"","Dir":"/home/ia_info_empty_gecos@example.com","Shell":"/bin/sh/ia_info_empty_gecos@example.com","Groups":[{"Name":"group-ia_info_empty_gecos@example.com","GID":null,"UGID":"ugid-ia_info_empty_gecos@example.com"}]}
data: {"userinfo":{"Name":"ia_info_empty_gecos@example.com","UID":0,"Gecos":"","Dir":"/home/ia_info_empty_gecos@example.com","Shell":"/bin/sh/ia_info_empty_gecos@example.com","Groups":[{"Name":"group-ia_info_empty_gecos@example.com","GID":null,"UGID":"ugid-ia_info_empty_gecos@example.com"}]}}
err: <nil>
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FIRST CALL:
access: granted
data: {"Name":"ia_info_empty_ugid@example.com","UID":0,"Gecos":"gecos for ia_info_empty_ugid@example.com","Dir":"/home/ia_info_empty_ugid@example.com","Shell":"/bin/sh/ia_info_empty_ugid@example.com","Groups":[{"Name":"group-ia_info_empty_ugid@example.com","GID":null,"UGID":""}]}
data: {"userinfo":{"Name":"ia_info_empty_ugid@example.com","UID":0,"Gecos":"gecos for ia_info_empty_ugid@example.com","Dir":"/home/ia_info_empty_ugid@example.com","Shell":"/bin/sh/ia_info_empty_ugid@example.com","Groups":[{"Name":"group-ia_info_empty_ugid@example.com","GID":null,"UGID":""}]}}
err: <nil>
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FIRST CALL:
access: granted
data: {"Name":"different_username@example.com","UID":0,"Gecos":"gecos for ia_info_mismatching_user_name@example.com","Dir":"/home/ia_info_mismatching_user_name@example.com","Shell":"/bin/sh/ia_info_mismatching_user_name@example.com","Groups":[{"Name":"group-ia_info_mismatching_user_name@example.com","GID":null,"UGID":"ugid-ia_info_mismatching_user_name@example.com"}]}
data: {"userinfo":{"Name":"different_username@example.com","UID":0,"Gecos":"gecos for ia_info_mismatching_user_name@example.com","Dir":"/home/ia_info_mismatching_user_name@example.com","Shell":"/bin/sh/ia_info_mismatching_user_name@example.com","Groups":[{"Name":"group-ia_info_mismatching_user_name@example.com","GID":null,"UGID":"ugid-ia_info_mismatching_user_name@example.com"}]}}
err: <nil>
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FIRST CALL:
access: granted
data: {"Name":"success@example.com","UID":0,"Gecos":"gecos for success@example.com","Dir":"/home/success@example.com","Shell":"/bin/sh/success@example.com","Groups":[{"Name":"group-success@example.com","GID":null,"UGID":"ugid-success@example.com"}]}
data: {"userinfo":{"Name":"success@example.com","UID":0,"Gecos":"gecos for success@example.com","Dir":"/home/success@example.com","Shell":"/bin/sh/success@example.com","Groups":[{"Name":"group-success@example.com","GID":null,"UGID":"ugid-success@example.com"}]}}
err: <nil>
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ FIRST CALL:
err: <nil>
SECOND CALL:
access: granted
data: {"Name":"ia_second_call@example.com","UID":0,"Gecos":"gecos for ia_second_call@example.com","Dir":"/home/ia_second_call@example.com","Shell":"/bin/sh/ia_second_call@example.com","Groups":[{"Name":"group-ia_second_call@example.com","GID":null,"UGID":"ugid-ia_second_call@example.com"}]}
data: {"userinfo":{"Name":"ia_second_call@example.com","UID":0,"Gecos":"gecos for ia_second_call@example.com","Dir":"/home/ia_second_call@example.com","Shell":"/bin/sh/ia_second_call@example.com","Groups":[{"Name":"group-ia_second_call@example.com","GID":null,"UGID":"ugid-ia_second_call@example.com"}]}}
err: <nil>
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
FIRST CALL:
access: granted
data: {"Name":"ia_second_call@example.com","UID":0,"Gecos":"gecos for ia_second_call@example.com","Dir":"/home/ia_second_call@example.com","Shell":"/bin/sh/ia_second_call@example.com","Groups":[{"Name":"group-ia_second_call@example.com","GID":null,"UGID":"ugid-ia_second_call@example.com"}]}
data: {"userinfo":{"Name":"ia_second_call@example.com","UID":0,"Gecos":"gecos for ia_second_call@example.com","Dir":"/home/ia_second_call@example.com","Shell":"/bin/sh/ia_second_call@example.com","Groups":[{"Name":"group-ia_second_call@example.com","GID":null,"UGID":"ugid-ia_second_call@example.com"}]}}
err: <nil>
SECOND CALL:
access: granted
data: {"Name":"ia_second_call@example.com","UID":0,"Gecos":"gecos for ia_second_call@example.com","Dir":"/home/ia_second_call@example.com","Shell":"/bin/sh/ia_second_call@example.com","Groups":[{"Name":"group-ia_second_call@example.com","GID":null,"UGID":"ugid-ia_second_call@example.com"}]}
data: {"userinfo":{"Name":"ia_second_call@example.com","UID":0,"Gecos":"gecos for ia_second_call@example.com","Dir":"/home/ia_second_call@example.com","Shell":"/bin/sh/ia_second_call@example.com","Groups":[{"Name":"group-ia_second_call@example.com","GID":null,"UGID":"ugid-ia_second_call@example.com"}]}}
err: <nil>
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
FIRST CALL:
access: granted
data: {"userinfo":{"Name":"ia_granted_with_data@example.com","UID":0,"Gecos":"gecos for ia_granted_with_data@example.com","Dir":"/home/ia_granted_with_data@example.com","Shell":"/bin/sh/ia_granted_with_data@example.com","Groups":[{"Name":"group-ia_granted_with_data@example.com","GID":null,"UGID":"ugid-ia_granted_with_data@example.com"}]},"message":"Offline login is enabled with your Entra password"}
err: <nil>
26 changes: 23 additions & 3 deletions internal/services/pam/pam.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,15 @@ func (s Service) IsAuthenticated(ctx context.Context, req *authd.IARequest) (res
}, nil
}

var uInfo types.UserInfo
if err := json.Unmarshal([]byte(data), &uInfo); err != nil {
var grantedData struct {
UserInfo types.UserInfo `json:"userinfo"`
Message string `json:"message,omitempty"`
}
if err := json.Unmarshal([]byte(data), &grantedData); err != nil {
log.Errorf(ctx, "IsAuthenticated: Could not unmarshal user data for session %q: %v", sessionID, err)
return nil, fmt.Errorf("user data from broker invalid: %v", err)
}
uInfo := grantedData.UserInfo

// authd uses lowercase user and group names
uInfo.Name = strings.ToLower(uInfo.Name)
Expand Down Expand Up @@ -315,9 +319,25 @@ func (s Service) IsAuthenticated(ctx context.Context, req *authd.IARequest) (res
return nil, err
}

// IAResponse.Msg always carries a JSON {"message": ...} envelope (or an
// empty string when there is no message), matching the format used for
// non-granted responses and expected by the PAM client's dataToMsg parser.
// A granted login must never depend on this purely cosmetic message, so we
// only fail here if the broker-supplied message cannot be re-encoded, which
// should not happen for a valid string.
msg := ""
if grantedData.Message != "" {
m, err := json.Marshal(map[string]string{"message": grantedData.Message})
if err != nil {
log.Errorf(ctx, "IsAuthenticated: Could not marshal granted message for session %q: %v", sessionID, err)
return nil, fmt.Errorf("could not marshal granted message: %v", err)
}
msg = string(m)
}

return &authd.IAResponse{
Access: access,
Msg: "",
Msg: msg,
}, nil
}

Expand Down
1 change: 1 addition & 0 deletions internal/services/pam/pam_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ func TestIsAuthenticated(t *testing.T) {
// There is no wantErr as it's stored in the golden file.
}{
"Successfully_authenticate": {username: "success@example.com"},
"Successfully_authenticate_with_granted_message": {username: "ia_granted_with_data@example.com"},
"Successfully_authenticate_if_first_call_is_canceled": {username: "ia_second_call@example.com", secondCall: true, cancelFirstCall: true},
"Denies_authentication_when_broker_times_out": {username: "ia_timeout@example.com"},
"Update_existing_DB_on_success": {username: "success@example.com", existingDB: "cache-with-user.db"},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
FIRST CALL:
access: granted
msg: {"message":"Offline login is enabled with your Entra password"}
err: <nil>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
users:
- name: ia_granted_with_data@example.com
uid: 1111
gid: 1111
gecos: gecos for ia_granted_with_data@example.com
dir: /home/ia_granted_with_data@example.com
shell: /bin/sh/ia_granted_with_data@example.com
groups:
- name: ia_granted_with_data@example.com
gid: 1111
ugid: ia_granted_with_data@example.com
- name: group-ia_granted_with_data@example.com
gid: 22222
ugid: ugid-ia_granted_with_data@example.com
users_to_groups:
- uid: 1111
gid: 1111
- uid: 1111
gid: 22222
schema_version: 2
8 changes: 8 additions & 0 deletions internal/testutils/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,14 @@ func (b *BrokerBusMock) IsAuthenticated(sessionID, authenticationData string) (a
access = authNext
data = `{"message": "It's fine to show a message here"}`

case "ia_granted_with_data":
access = authGranted
data = fmt.Sprintf(`{"userinfo": %s, "message": "Offline login is enabled with your Entra password"}`, userInfoFromName(sessionID, nil))

case "ia_granted_with_non_string_message":
access = authGranted
data = fmt.Sprintf(`{"userinfo": %s, "message": 42}`, userInfoFromName(sessionID, nil))

case "ia_next_with_invalid_data":
access = authNext
data = `{"msg": "there should not be a message here"}`
Expand Down
14 changes: 11 additions & 3 deletions pam/internal/adapter/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,11 +352,19 @@ func (m authenticationModel) Update(msg tea.Msg) (authModel authenticationModel,

var authMsg string
if msg.access != auth.Cancelled {
msg, err := dataToMsg(msg.msg)
parsedMsg, err := dataToMsg(msg.msg)
if err != nil {
return m, sendEvent(pamError{status: pam.ErrSystem, msg: err.Error()})
// A malformed or unexpected message must never fail an
// authentication that the broker has already granted: the
// message is a purely cosmetic notice. For any other access we
// still surface the error.
if msg.access != auth.Granted {
return m, sendEvent(pamError{status: pam.ErrSystem, msg: err.Error()})
}
log.Warningf(context.TODO(), "Ignoring invalid granted message: %v", err)
} else {
authMsg = parsedMsg
}
authMsg = msg
}

switch msg.access {
Expand Down
6 changes: 6 additions & 0 deletions pam/internal/adapter/gdmmodel.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,12 @@ func (m gdmModel) Update(msg tea.Msg) (gdmModel, tea.Cmd) {
case gdmIsAuthenticatedResultReceived:
access := msg.access
authMsg, err := dataToMsg(msg.msg)
if err != nil && access == auth.Granted {
// A malformed granted message must never fail an already-granted
// authentication; it is a purely cosmetic notice.
log.Warningf(context.TODO(), "Ignoring invalid granted message: %v", err)
authMsg, err = "", nil
}
if err != nil {
return m, sendEvent(pamError{status: pam.ErrSystem, msg: err.Error()})
}
Expand Down
46 changes: 46 additions & 0 deletions pam/internal/adapter/gdmmodel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,52 @@ func TestGdmModel(t *testing.T) {
msg: "Hi GDM, it's a pleasure to get you in!",
},
},
"Authenticated_with_invalid_message_still_succeeds_with_preset_PAM_user_and_server_side_broker_and_authMode_selection": {
clientOptions: append(slices.Clone(multiBrokerClientOptions),
pam_test.WithGetPreviousBrokerReturn(firstBrokerInfo.Id, nil),
pam_test.WithIsAuthenticatedReturn(&authd.IAResponse{
Access: auth.Granted,
// A bare (non-JSON) message must never fail an
// already-granted authentication: it is dropped, not fatal.
Msg: "You're in, but this is not a valid JSON envelope!",
}, nil),
),
pamUser: "pam-preset-user-and-daemon-selected-broker",
messages: []tea.Msg{
gdmTestWaitForStage{
stage: proto.Stage_challenge,
commands: []tea.Cmd{
sendEvent(gdmTestSendAuthDataWhenReady{&authd.IARequest_AuthenticationData_Secret{
Secret: "gdm-good-password",
}}),
},
},
},
wantSelectedBroker: firstBrokerInfo.Id,
wantGdmRequests: []gdm.RequestType{
gdm.RequestType_uiLayoutCapabilities,
gdm.RequestType_changeStage, // -> broker Selection
gdm.RequestType_changeStage, // -> authMode Selection
gdm.RequestType_changeStage, // -> password
},
wantGdmEvents: []gdm.EventType{
gdm.EventType_userSelected,
gdm.EventType_brokersReceived,
gdm.EventType_brokerSelected,
gdm.EventType_authModeSelected,
gdm.EventType_uiLayoutReceived,
gdm.EventType_authEvent,
gdm.EventType_startAuthentication,
},
wantStage: proto.Stage_challenge,
wantGdmAuthRes: []*authd.IAResponse{{
Access: auth.Granted,
Msg: "",
}},
wantExitStatus: PamSuccess{
BrokerID: firstBrokerInfo.Id,
},
},
"New_password_changed_after_server_side_broker_and_authMode_selection": {
clientOptions: append(slices.Clone(singleBrokerNewPasswordClientOptions),
pam_test.WithGetPreviousBrokerReturn(firstBrokerInfo.Id, nil),
Expand Down
Loading
Loading