Support for JWT authentication mode in telemetry #152
Conversation
saravanan-i
changed the title
|
/azpw run sonic-gnmi |
|
/AzurePipelines run Azure.sonic-buildimage |
|
Commenter does not have sufficient privileges for PR 152 in repo sonic-net/sonic-gnmi |
| var cmd string | ||
| cmd = "user_auth_mgmt.retrieve_user_roles" |
There was a problem hiding this comment.
Those two lines can be combined in just one:
cmd := "user_auth_mgmt.retrieve_user_roles"
| host_output := transformer.HostQuery(cmd, username) | ||
| if host_output.Err != nil { | ||
| glog.Errorf("System user roles host query failed") | ||
| return nil,errors.New("Failed to retrieve user roles") |
There was a problem hiding this comment.
Please add a space after the comma.
| if val, _ := host_output.Body[0].(int32); val == 0 { | ||
| glog.Infof("Roles retrieved from host") | ||
| roles := strings.Split(host_output.Body[1].(string), ",") | ||
| return roles,nil |
There was a problem hiding this comment.
Please add a space after the comma.
| return roles,nil | ||
| } else { | ||
| glog.Errorf("Invalid User. no roles") | ||
| return nil,errors.New(host_output.Body[1].(string)) |
There was a problem hiding this comment.
Please add a space after the comma.
| } | ||
|
|
||
| roles, err := GetUserRoles(usr) | ||
| roles,err := GetUserRoles(username) |
There was a problem hiding this comment.
Please add a space after the comma.
| clientTypes = flags.NewStringList(&cfg.ClientTypes, []string{gclient.Type}) | ||
| queryFlag = &flags.StringList{} | ||
| - queryType = flag.String("query_type", client.Once.String(), "Type of result, one of: (o, once, p, polling, s, streaming).") | ||
| + queryType = flag.String("query_type", "", "Type of result, one of: (p, polling, s, streaming).") |
There was a problem hiding this comment.
Why not to specify the default mode? Say, polling?
| if srv.config.UserAuth.Enabled("jwt") { | ||
| log.V(1).Info("gNOI: Sonic Authenticate - JWT enabled") | ||
| } else { | ||
| log.V(1).Info("gNOI: Sonic Authenticate - JWT not enabled" |
There was a problem hiding this comment.
syntax error: missing ) at the end
| @@ -159,6 +157,7 @@ func (srv *Server) Refresh(ctx context.Context, req *spb_jwt.RefreshRequest) (*s | |||
|
|
|||
| token, _, err := JwtAuthenAndAuthor(ctx) | |||
| if err != nil { | |||
| log.Errorf("gNOI: Sonic Refresh - JWTAuthenandAuthor returned error") | |||
There was a problem hiding this comment.
Please add information what error was returned.
| glog.Errorf("System user roles host query failed") | ||
| return nil,errors.New("Failed to retrieve user roles") | ||
| } else { | ||
| if val, _ := host_output.Body[0].(int32); val == 0 { |
There was a problem hiding this comment.
Please do not ignore errors. The condition can look like follows:
if val, ok := host_output.Body[0].(int32); ok && val == 0 {
| } else { | ||
| if val, _ := host_output.Body[0].(int32); val == 0 { | ||
| glog.Infof("Roles retrieved from host") | ||
| roles := strings.Split(host_output.Body[1].(string), ",") |
There was a problem hiding this comment.
Please do not ignore errors. If Body[1] is not a string then the program will panic. Even if this not normally/today possible it might change in the future.
Why I did it
1)To support JWT client authentication in telemetry dialin mode.
2) Patched gnmi_cli to have only supported streaming options like polling and streaming
How I did it
How to verify it
Which release branch to backport (provide reason below if selected)
Description for the changelog
Added methods to enable JWT token support in telemetry and patched gnmi_cli to show only the supported streaming types
Dependent Merge Requests:
sonic-net/sonic-buildimage#13142
sonic-net/sonic-host-services#76
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)