Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polishing some aspects of the application #11

Merged
merged 11 commits into from Jun 7, 2022
Merged

Polishing some aspects of the application #11

merged 11 commits into from Jun 7, 2022

Conversation

rayoz12
Copy link
Collaborator

@rayoz12 rayoz12 commented May 11, 2022

Goals of PR:

1. Move Field management back to OpenHarvest

2. Implement viewing a farmer's details of their fields

3. Improve the Map Experience by replacing Latlng text fields with a new point selector component

4. Enable JWT based Authentication to OpenHarvest as a standard.

  • There are a few reasons for this:
  • Integrations are unable to use the IBMid OAuth, OpenID authentication flow without having to open a browser and authenticate through OpenHarvest
  • We will need to support API Key based authentication to make it easier to interface with OpenHarvest from Integrations. Part of an overall strategy of making the Integration Experience as easy and slick as it can be.
  • We are currently using session based authentication which can't be scaled to multiple services. If we ever need OpenHarvest authentication in another OpenHarvest service it will make things difficult.
    • We can of course use a database to store session state but it feels like too much for just authentication.

Authentication Flow

sequenceDiagram
    participant client as Client (Web App)
    participant api as OpenHarvest API
    participant ibmid as IBMId
    client->>api: /auth/login
    api-->>client: 302 Redirect to IBMId for Auth
    client->>ibmid: Authenticates using
    ibmid->>api: "/auth/sso/callback": With user details 
    api->client: Updates session with user details
    api->client: Generates a JWT Token for user
    api->>client: "/?token=..." attaches token to a http param
    opt Get User Details
        client->>api: Req. for User details: Token in "Authorization" header
        api->>client: User Details
    end
    opt API request
        client->>api: Call OpenHarvest API: Token in "Authorization" header
    end

@rayoz12 rayoz12 changed the title Polishing some aspects of the application WIP: Polishing some aspects of the application May 11, 2022
@vikasjagtap
Copy link
Collaborator

(Just looking at the auth flow digram): I think token should be sent in the header, we should introduce at least two custom headers X-OPENHARVEST-TOKEN and X-OPENHARVEST-ORG, the token might be able to hold organisation of the user, but that will impact the user's login flow at the frontend, so we should have both if it makes more flexible.

@rayoz12
Copy link
Collaborator Author

rayoz12 commented May 16, 2022

(Just looking at the auth flow digram): I think token should be sent in the header, we should introduce at least two custom headers X-OPENHARVEST-TOKEN and X-OPENHARVEST-ORG, the token might be able to hold organisation of the user, but that will impact the user's login flow at the frontend, so we should have both if it makes more flexible.

I agree on sending both in the header. For the token I'm using the standard Authorization header as a bearer token. What do you mean by impacting the user's login flow?

What happens right now is that the user is on boarded (with their selected org) and the web app calls the /me endpoint to get the updated user details. Since the payload in both the token and this endpoint is the same we can just use what that endpoint returns from then on.

Edit: Updated the Diagram above to show what I mean.

rayoz12 and others added 11 commits June 7, 2022 11:15
Onboarding is now shown correctly when the user first logs in
The nav menu hides options when the user isn't logged in.
Converted App to a function
Added an isLoggedIn prop to the auth object

Signed-off-by: Ryan Pereira <rayoz12@yahoo.com.au>
Added Point Selector Layer
Onboarding now uses point selector layer to set a location

Signed-off-by: Ryan Pereira <rayoz12@yahoo.com.au>
Signed-off-by: Ryan Pereira <rayoz12@yahoo.com.au>
Integrate JWT Auth

Signed-off-by: Ryan Pereira <rayoz12@yahoo.com.au>
Signed-off-by: Ryan Pereira <rayoz12@yahoo.com.au>
Signed-off-by: Ryan Pereira <rayoz12@yahoo.com.au>
Signed-off-by: Ryan Pereira <rayoz12@yahoo.com.au>
Signed-off-by: Ryan Pereira <rayoz12@yahoo.com.au>
- Map now has the ability to be centred and bounds aligned.

Signed-off-by: Ryan Pereira <rayoz12@yahoo.com.au>
Signed-off-by: Ryan Pereira <rayoz12@yahoo.com.au>
@rayoz12 rayoz12 merged commit 387ff36 into master Jun 7, 2022
@rayoz12 rayoz12 changed the title WIP: Polishing some aspects of the application Polishing some aspects of the application Jun 7, 2022
@rayoz12
Copy link
Collaborator Author

rayoz12 commented Jun 7, 2022

Merged so @JonathanScialpi can start his work

@rayoz12 rayoz12 deleted the ryan-polish branch July 10, 2022 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants