App service support for MI #537
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import ( | |
| "errors" | ||
| "fmt" | ||
| "reflect" | ||
| "strconv" | ||
| "strings" | ||
| "time" | ||
|
|
||
|
|
@@ -173,14 +174,80 @@ type TokenResponse struct { | |
| FamilyID string `json:"foci"` | ||
| IDToken IDToken `json:"id_token"` | ||
| ClientInfo ClientInfo `json:"client_info"` | ||
| ExpiresOn internalTime.DurationTime `json:"-"` | ||
| ExtExpiresOn internalTime.DurationTime `json:"ext_expires_in"` | ||
| GrantedScopes Scopes `json:"scope"` | ||
| DeclinedScopes []string // This is derived | ||
|
|
||
| AdditionalFields map[string]interface{} | ||
| scopesComputed bool | ||
| } | ||
|
|
||
| func (tr *TokenResponse) UnmarshalJSON(data []byte) error { | ||
| type Alias TokenResponse | ||
| aux := &struct { | ||
| ExpiresIn json.Number `json:"expires_in,omitempty"` | ||
| ExpiresOn interface{} `json:"expires_on,omitempty"` | ||
| *Alias | ||
| }{ | ||
| Alias: (*Alias)(tr), | ||
| } | ||
|
|
||
| // Unmarshal the JSON data into the aux struct | ||
| if err := json.Unmarshal(data, &aux); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| parseDuration := func(num json.Number) (int64, error) { | ||
|
There was a problem hiding this comment. How is this better than |
||
| if num == "" { | ||
| return 0, nil | ||
| } | ||
| return num.Int64() | ||
| } | ||
|
|
||
| // Function to parse different date formats | ||
| parseExpiresOn := func(expiresOn string) (time.Time, error) { | ||
|
There was a problem hiding this comment. Pls add a comment pointing at AzureAD/microsoft-authentication-library-for-dotnet#4963 to explain when this can occur. There was a problem hiding this comment. Maybe I'm overlooking it, but that issue doesn't mention the platform returning the unsupported format There was a problem hiding this comment. The CX who reported this didn't make it very clear, it seems to be container + App Service. There was a problem hiding this comment. It would be good to know what the linked PR added support for so that support is easier to maintain. Setting that aside, the linked PR added ISO 8601 parsing. Why does this PR have two additional formats? There was a problem hiding this comment. We asked, the CX didn't answer. Ultimately, Azure SDK for .NET did not have this problem so it was clearly a regression. Not objecting to supporting only ISO 8601. The .NET code uses a general-purpose DateTime parser btw https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/4964/files#diff-a7d2338916806e48fb637fd9caad6095fec8537a96d157bc2ea47ab64903fef9R93 I assume Azure.SDK for .NET did the same. |
||
| var formats = []string{ | ||
| "01/02/2006 15:04:05", // MM/dd/yyyy HH:mm:ss | ||
| "2006-01-02 15:04:05", // yyyy-MM-dd HH:mm:ss | ||
| time.RFC3339Nano, // ISO 8601 (with nanosecond precision) | ||
| } | ||
|
|
||
| for _, format := range formats { | ||
| if t, err := time.Parse(format, expiresOn); err == nil { | ||
| return t, nil | ||
| } | ||
| } | ||
| return time.Time{}, fmt.Errorf("invalid ExpiresOn format: %s", expiresOn) | ||
| } | ||
|
|
||
| if expiresOnStr, ok := aux.ExpiresOn.(string); ok { | ||
| if ts, err := strconv.ParseInt(expiresOnStr, 10, 64); err == nil { | ||
| tr.ExpiresOn = internalTime.DurationTime{T: time.Unix(ts, 0)} | ||
| return nil | ||
| } | ||
| if expiresOnStr != "" { | ||
| if t, err := parseExpiresOn(expiresOnStr); err != nil { | ||
| return err | ||
| } else { | ||
| tr.ExpiresOn = internalTime.DurationTime{T: t} | ||
| return nil | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Check if ExpiresOn is a number (Unix timestamp or ISO 8601) | ||
| if expiresOnNum, ok := aux.ExpiresOn.(float64); ok { | ||
| tr.ExpiresOn = internalTime.DurationTime{T: time.Unix(int64(expiresOnNum), 0)} | ||
| return nil | ||
| } | ||
| if duration, err := parseDuration(aux.ExpiresIn); err != nil { | ||
| return err | ||
| } else if duration > 0 { | ||
| tr.ExpiresOn = internalTime.DurationTime{T: time.Now().Add(time.Duration(duration) * time.Second)} | ||
| return nil | ||
| } | ||
| return errors.New("expires_in and expires_on are both missing or invalid") | ||
| } | ||
|
|
||
| // ComputeScope computes the final scopes based on what was granted by the server and | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DurationTime exists to calculate ExpiresOn from
expires_in. That doesn't help now that you want to consider a possible value ofexpires_onas well, and you're disabling unmarshaling for this field anyway. So, why not make the type here simplytime.Time?