Skip to content

Conversation

@heebinho
Copy link
Contributor

@heebinho heebinho commented Jun 6, 2022

Finally I found some time to work on the FileMakerCloud Authentication. Some thoughts regarding this pull request:

  • As you have suggested, I've placed the aws packages in a separate project with the name FMData.Rest.Auth.FileMakerCloud
  • I thought it's important to have the "full marketing name" at the end (FileMakerCloud), so that newcomers will easily understand the principle and that "following" implementations (Google, Microsoft Azure AD) will/can fall in place.
  • The AWSSDK.CognitoIdentityProvider is not compatible with netstandard1.3 - this is why I have removed it from the target frameworks in the new project
  • The Amazon.Extensions.CognitoAuthentication is not compatible with net45 - this is why I have removed it from the target frameworks in the new project
  • I have added a new ctor in FileMakerRestClient, which is used like:
    new FileMakerRestClient(new HttpClient(), new FileMakerCloudAuthTokenProvider(conn));

What do you think? I'm happy to discuss and improve.
Currently I do have access to a FileMakerCloud account and can test.

regards,

Renato

@fuzzzerd fuzzzerd self-assigned this Jun 9, 2022
@fuzzzerd fuzzzerd added enhancement New feature or request data-api Pertaining to the Data API. breaking change labels Jun 9, 2022
@fuzzzerd
Copy link
Owner

fuzzzerd commented Jun 9, 2022

Thank you for the pull request! I will review soon.

Copy link
Owner

@fuzzzerd fuzzzerd left a comment

Choose a reason for hiding this comment

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

LGTM!

@fuzzzerd fuzzzerd added this to the v5 milestone Aug 2, 2022
@fuzzzerd fuzzzerd merged commit f12a3f2 into fuzzzerd:master Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change data-api Pertaining to the Data API. enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants