Skip to content

Conversation

ncooke3
Copy link
Member

@ncooke3 ncooke3 commented May 12, 2021

b/187427651

TODO:

  • Initial implementation
  • Finish and commit unit tests
  • Maybe: Refactor after some initial feedback

#no-changelog

@ncooke3 ncooke3 force-pushed the nc/exchange-assertion-request branch from 1ef7c86 to 70cda5f Compare May 12, 2021 17:00
static NSString *const kRequestFieldKeyID = @"key_id";
static NSString *const kRequestFieldArtifact = @"artifact";
static NSString *const kRequestFieldAssertion = @"assertion";
static NSString *const kRequestFieldAttestation = @"attestationStatement";
Copy link
Contributor

Choose a reason for hiding this comment

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

This is something we will need to double check because the DeviceCheck endpoint accepts snake case field names.


static NSString *const kExchangeAppAttestAssertionEndpoint = @"exchangeAppAttestAssertion";
static NSString *const kExchangeAppAttestAttestationEndpoint = @"exchangeAppAttestAttestation";
static NSString *const kGenerateAppAttestChallengeEndpoint = @"generateAppAttestChallenge";
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

[NSString stringWithFormat:@"%@/projects/%@/apps/%@:generateAppAttestChallenge",
self.APIService.baseURL, self.projectID, self.appID];
NSURL *URL = [NSURL URLWithString:URLString];
NSURL *URL = [self URLForEndpoint:kGenerateAppAttestChallengeEndpoint];
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 nice cleanup!

Comment on lines 223 to 229
- (id)HTTPBodyWithJSONObject:(nonnull id)JSONObject {
NSError *encodingError;
NSData *payloadJSON = [NSJSONSerialization dataWithJSONObject:JSONObject
options:0
error:&encodingError];
return payloadJSON ?: [FIRAppCheckErrorUtil JSONSerializationError:encodingError];
}
Copy link
Member Author

@ncooke3 ncooke3 May 12, 2021

Choose a reason for hiding this comment

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

Added this helper to hopefully consolidate the class's JSON serialization logic. WDYT? Returning id to allow for either NSData or NSError but could instead return a promise that has been fulfilled/rejected...

Copy link
Contributor

Choose a reason for hiding this comment

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

A fulfilled/rejected promise looks like a much more readable option to me.

Copy link
Contributor

@maksymmalyhin maksymmalyhin left a comment

Choose a reason for hiding this comment

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

👍 Mostly LGTM, a couple comments.

Comment on lines 223 to 229
- (id)HTTPBodyWithJSONObject:(nonnull id)JSONObject {
NSError *encodingError;
NSData *payloadJSON = [NSJSONSerialization dataWithJSONObject:JSONObject
options:0
error:&encodingError];
return payloadJSON ?: [FIRAppCheckErrorUtil JSONSerializationError:encodingError];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

A fulfilled/rejected promise looks like a much more readable option to me.

body:HTTPBody
additionalHeaders:@{kContentTypeKey : kJSONContentType}];
})
// TODO: Move to default queue.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Member Author

Choose a reason for hiding this comment

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