/api/oauth/token must POST secrets as x-www-form-urlencoded body instead of in URL parameters

[vorburger]

As per https://issues.apache.org/jira/browse/FINERACT-629,

though shall kindly change https://github.com/openMF/web-app/blob/master/src/app/core/authentication/authentication.service.ts#L96,

to make sure that it doesn't pass any arguments to /oauth/token as HTTP parameters, but instead only POSTs them in a x-www-form-urlencoded body instead,

as shown in the example in https://issues.apache.org/jira/browse/FINERACT-1145.

@edcable will you make sure this gets done ASAP?

[vorburger]

https://github.com/apache/fineract/pull/1320/files probably illustrates best what actually needs to be changed how (I understand this proejct doesn't use JQuery to invoke the REST API, but you get the idea and should be able to do the equivalent).

[Abhirup-99]

@vorburger I would like to work on this issue.

[vorburger]

@Abhirup-99 then please do! ;-)

[Abhirup-99]

I have 2 open prs, can you review them. They are ready and hopefully can be merged. @vorburger

[vorburger]

I have 2 open prs, can you review them. They are ready and hopefully can be merged. @vorburger

https://github.com/openMF/web-app/pulls/Abhirup-99 currently only shows #1243 and #1246 from you, and neither seem to have anything to do with this, so I'm afraid I'm not following what you mean here; sorry. -- FYI I'm not an active maintainer of this repo (I personally focus on https://github.com/apache/fineract/), but I trust that those who maintain this project (I've not followed along much recently who does, but looking at https://github.com/openMF/web-app/commits/master can see that the project is fairly active - great), when you raise a PR related to this issue, will review it and merge it.

[Abhirup-99]

I have 2 open prs, can you review them. They are ready and hopefully can be merged. @vorburger

https://github.com/openMF/web-app/pulls/Abhirup-99 currently only shows #1243 and #1246 from you, and neither seem to have anything to do with this, so I'm afraid I'm not following what you mean here; sorry. -- FYI I'm not an active maintainer of this repo (I personally focus on https://github.com/apache/fineract/), but I trust that those who maintain this project (I've not followed along much recently who does, but looking at https://github.com/openMF/web-app/commits/master can see that the project is fairly active - great), when you raise a PR related to this issue, will review it and merge it.

Thanks for the message. I would push a pr on 1st October, sorry for the delay as this would count towards Hackoctoberfest.

[Abhirup-99]

@vorburger I have updated the authentication, can you take a look at the pr. Thanks.

[Abhirup-99]

I revisited this issue to take a look at what can be done to resolve it. With the current implementation, OAuth won't have worked, regardless of whether it would have been sent as urlencoded or as GET parameters. There are 3 current issues with the code.

1. Below is the curl request that would have worked

   curl [--insecure] --location --request POST \
   'https://localhost:8443/fineract-provider/api/oauth/token' \
   --header 'Fineract-Platform-TenantId: default' \
   --header 'Content-Type: application/x-www-form-urlencoded' \
   --data-urlencode 'username=mifos' \
   --data-urlencode 'password=password' \
   --data-urlencode 'client_id=community-app' \
   --data-urlencode 'grant_type=password' \
   --data-urlencode 'client_secret=123'

   whereas this is the current code before my implementation

     bodyData = bodyData.set('client_id', 'community-app');
     bodyData = bodyData.set('grant_type', 'password');
     bodyData = bodyData.set('client_secret', '123');

2. there is a race around condition due to which the first 2 requests at the notifications endpoint always fails.

     if (environment.oauth.enabled) {
       this.refreshOAuthAccessToken();
     } else {
       authenticationInterceptor.setAuthorizationToken(savedCredentials.base64EncodedAuthenticationKey);
     }

   for no oauth enabled calls, the Authorization Token is set immediately whereas incase of it being enabled, the token is set in this.refreshOAuthAccessToken() function which @karantakalkar pointed out in the pr.

3. refreshOAuthAccessToken() function would never work because it tries to refresh the token by making a call to /api/oauth/token endpoint but it would never have access to the password of the user because the function is called when the user has clicked remember me, so on subsequent visits to the web app.

Current Workaround

• My proposition is to not alter the entire Auth flow too much but to remove the refreshOAuthToken function in its entirety and the user would automatically be logged out when the OAuth token expires, while this won't be completely gracious in the sense that on some occasions the user would only get the alert that profile not identified which is currently shown in case verification of the user fails, but this won't hamper the flow too much. We can provide some UI in a later stage directing the user to sign in again.
• The other alternative to keep the entire current flow is to save the password of the user in local storage. This can prove to be a great security risk and again keeping in mind this is the FE of a banking app I am not in favor of it.

@karantakalkar @vorburger do take a look.