The First Scenario: Authenticated orders (a minimal authentication)

[nabdelgadir]

Description

Step 3 from #1035 (comment).

Rework UserOrderController to obtain the customer id from the request (the access token provided by the client) instead of a URL-path parameter.

For example, we can remove /users/{userId} prefix from all endpoints to end up with the following API:

• POST /orders creates a new order
• GET /orders returns all orders of the current user
• PATCH /orders?where= to update some of the orders of the current user. I think this endpoint does not make sense in a Shopping app and should be eventually removed.
• DELETE /orders?where= to delete some of the orders of the current user. I think this endpoint should be eventually removed, because orders are never deleted, they can be only closed (e.g. as cancelled).

Existing REST API exposed by UserController should remain unauthenticated (allowing anonymous access).

The goal of this story is to leverage @loopback/authentication to implement this feature and improve @loopback/authentication along the way as needed.

Previous step: #1997
Next step: #1999

[dhmlau]

Originated from #1035 (comment)

Acceptance Criteria

• Implement the "verify" api in the authentication provider. i.e. accept access token. and tell you whether the token is valid plus user info
  1. custom sequence action - authenticate. This will verify the access token and bind the security context in the context.
  2. well-known binding key for DI for the security context.

After this task is done, we should be able to support these APIs:
POST /orders creates a new order
GET /orders returns all orders of the current user

See @loopback/authentication as inspiration.

[jannyHou]

Hi @raymondfeng @bajtos I plan to work on this story while realize PR loopbackio/loopback4-example-shopping#26 already covers the acceptance criteria as the created custom authentication action

Are we good to close this task?

After this task is done, we should be able to support these APIs:
POST /orders creates a new order
GET /orders returns all orders of the current user

And then create another story for ^

[bajtos]

At the moment, UserOrderController allow clients to impersonate any user, see e.g.

https://github.com/strongloop/loopback4-example-shopping/blob/3cbb0a8b8d41a6db5c5a9ecafa78b3aef6b1aad3/src/controllers/user-order.controller.ts#L38-L49

export class UserOrderController {
  // ...

  async createOrder(
    @param.path.string('userId') userId: string,
    @requestBody() order: Order,
  ): Promise<Order> {
    if (userId !== order.userId) {
      throw new HttpErrors.BadRequest(
        `User id does not match: ${userId} !== ${order.userId}`,
      );
    }
    delete order.userId;
    return await this.userRepo.orders(userId).create(order);
  }
}

I find it ugly for an application to have authentication in place and yet allow callers to make orders for other users, it's kind of a tech debt for an example app like the shopping app.

In my plan outlined in #1035 (comment), the goal of the step The First Scenario: Authenticated orders (a minimal authentication) was to improve this part of our example.

The part about improving @loopback/authentication package is important as well.

The goal of this story is to leverage @loopback/authentication to implement this feature and improve @loopback/authentication along the way as needed.

I would like to see both these changes happen before we move on to next steps on this journey.

Planning wise, I am not sure what exactly was considered as the scope for this milestone and what not. Personally, I'd prefer to keep this story open until all changes described at the top are finished. However, I see that the acceptance criteria in #1998 (comment) describe a smaller scope, so maybe it's ok to create new GitHub stories for the remaining changes and close this one as done.

I'll leave it up to you @jannyHou and @dhmlau to decide.

For me, the most important part is to ensure all we implement all changes needed to make authentication easy to use and to make the example app to a high-quality reference implementation.

[dhmlau]

@jannyHou @bajtos , I'm good with @jannyHou 's proposal in here. About the task to not "allow callers to make orders for other users", either fixing it in this task or opening a new task is good with me.

[raymondfeng]

The discussion here illustrates the difference between authentication and authorization. For the order apis, authentication tells us who (such as John) is making the call and authorization should control if the user can create/fetch (Mary's) orders.

[dhmlau]

Discussed with @raymondfeng @jannyHou @emonddr:

• if /users/me is working, then making /users/{userid}/orders available can easily be done. If that's the case, we can defer this item from Q1.

@jannyHou , could you please verify? Thanks.

[dhmlau]

Acceptance Criteria

• verify /users/me is working properly today
• stretch goal:
  • remove /users/{userid} in the endpoints so that we have POST /orders, GET /orders (like in the original description)
    • Can be deferred to Q2.
    • we can inject the current user into the controller so that it becomes the property of the controller instance. (inject once in the class level)

[lygstate]

Yeap /users/me works today, but not works on other endpoints, such as orders

[dhmlau]

Related PR: loopbackio/loopback4-example-shopping#71

[dhmlau]

Closing per the discussion in https://github.com/strongloop/loopback-next/pull/4144/files#r347996012