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

Security concept for guest account #271

Closed
prinzdezibel opened this issue Jan 22, 2015 · 5 comments
Closed

Security concept for guest account #271

prinzdezibel opened this issue Jan 22, 2015 · 5 comments
Assignees

Comments

@prinzdezibel
Copy link
Contributor

ATM, a guest account is a completely open and unsecure thing.

Everybody can create a new login token with any email address he likes to. With that token he can view all the data assciated with this account:

Could be:

  • Transaction history
  • Orders
  • Shipment addresses
  • etc, etc

We need a concept how to deal with that. Some quick flash-lights:

  • Don't publish any data to the client that is connected to a guest account

  • eMail verification of created token

    a) Every time the user logs in (as guest)?
    b) Only The first time? But then, how to be sure that the user has the given email address the second time anybody tries to log in as guest with that email address?
    c) Use the login token from users browser for subsequent guest logins? (At the moment we just create a new guestLoginToken)

@aaronjudd
Copy link
Contributor

I think a guest account should have access to any "order" data, but without a password set it's just the specific order confirmation (not general account data), and only on while on a browser that has a valid guest token for the created order.

We should prompt to create a password on the order confirmation page (and that would finish converting the guest to an account).

We could send a login link to guest account that sets browser + session token, to review order details. (something like: check your order status here: link). This could be just a link in an order confirmation email. Whatever page it links to should have the opportunity to convert to an account by setting a password. Not sure if this should be tied to the specific computer/ip that created the order or not (if the email has been forwarded, can that person set the password, or lean towards the user might not care and has intentionally forwarded, and would rather see on a various machines).

If a user attempts to sign in to the guest account, or use the email to create an account that doesn't have a password, email them a set password link.

Without an account, the user should not be able to change order details or any other account information.

@prinzdezibel
Copy link
Contributor Author

I haven't completely digested it yet. This is the situation: Person A does checkout as guest with eMail [email protected] and ends with "order placed". Person B does also login as guest with the same eMail [email protected]. They both get a different guest token, but both tokens are tied to the same account. Therefore Person B may read the placed order from Person A.

Maybe it would be a good thing to tie the Order object to the login token for guest logins?

@aaronjudd Does this match your idea above or were you thinking differently?

@aaronjudd
Copy link
Contributor

I think I have another (additional) approach. I'm suggesting that we modify the cart methods, and specifically copyCartToOrder to not validate the user Id, a session id being enough.
then add functionality to:

    ###
    # todo: implement guest checkout here.
    # save sessionId to cart collection
    # if this guest sessionId becomes a user
    # or if an email sent to the user confirms sessionId belongs to this email
    # it can be reconnected to any user account where the email is registered.
    # we won't validate user account here further
    ###

Or maybe to resolve your "They both get a different guest token, but both tokens are tied to the same account." problem (yeah, that's valid).. maybe there isn't a way for a user to connect accounts unless they register on the same machine. The sessionId could still be there for admin actions on that, or other processing later.

@aldeed
Copy link
Contributor

aldeed commented Feb 12, 2015

Just reading through all these discussions again, and I'm wondering if you're overcomplicating things by trying to fit a square guest into a round user. It was my first inclination, too, but the various possibilities for confusion seem scary, from a security and maintainability perspective.

Here's my alternative thought:

  1. Add optional email property to cart/order doc.
  2. If checking out as guest, store the email there. userId property is blank.
  3. Wherever code would otherwise look at the user doc (like when sending order status emails, etc.), use the order email prop if there isn't a userId.
  4. When someone creates a user account, after they've verified their email address, look for that email in email prop on any past orders/carts. Clear the email prop and set the userId to the new userId.
  5. After a guest order is placed, there is no linking token, no logged in state, etc. The browser cache is wiped such that there is no way to see anything about that order you just placed.
  6. But there is some screen where you can request order details/status be emailed to you based on entering just an email address.

This might be similar to what @aaronjudd proposed in the previous comment, but I'm proposing that there is a point during guest checkout (maybe not until final submission, or maybe once address/payment info is entered, if we are concerned about the security of that info?) where we can forget sessionId and key off order email instead, without any user doc involved.

@prinzdezibel
Copy link
Contributor Author

Agree with you, @aldeed. Seems like the guest role in this context just isn't a good match to be modeled as user doc.

@aaronjudd aaronjudd self-assigned this Feb 26, 2015
aaronjudd added a commit that referenced this issue Mar 24, 2015
Thanks to contributions from @danielgindi `right to left internationalization` is now supported.

Take a look at the Russian or Hebrew languages to see RTL in action.

 - Resolves #176

Also contributions from  @firstred and @epson121 added Dutch and Croatian translations.

Thanks to contributions from @lovetostrike, the initial `reactioncommerce:reaction-social` package has been published, which gives basic social sharing functionality.

 - Resolves #61
 - Resolves #281

- if there is neither a sessionCart nor a userCart, create a sessionCart
  * add sessionId to cart.sessions
  * auto insert/update userId if authenticated
  * observers to ensure cart always exists (ie after deletion)
- if sessionCart is a not a userCart return sessionCart
- if sessionCart just authenticated, add userId
- only return a user cart when authenticated
  * copy existing userCart(s) into sessionCart and remove userCart(s)
- if userCart just logged out, remove sessionId from userCart and create new sessionCart
- allow multiple sessionId per cart when userCart

TODO:

- realistically this may not be very solid way to handle sessions.
- Session handling will need a **close review in the future**.
- potential edge cases where multiple session/auth/merge actions to a cart may destroy the cart
- cart really should have mirrored client and server methods and be tested offline

Issue updates:

- Remove packages introduced in reactioncommerce/reaction-core#76
- Resolves #271
- Resolves #339
- Resolves #326
- Resolves #183
- Strategic updates for issue #16
- Strategic updates for issue #318
- Strategic updates for issue #76

   * dashboard enable/disable **guest checkout**
   * change default step to **guest checkout** or create/sign-in
   * store email for order - **after order completion if guest**
   * not visible after destroyed session (ie: refresh)
   * updated checkout and progress bar to use
     cart value instead of session

TODO:

- validation on email guest entry
- add account creation instructions to an order completion email
- potentially add third prompt for account creation after email (or promos,etc)
- order emails!
- use workflow states to control checkout ui flow.

Thanks to initial contributions from @prinzdezibel!

- use new Accounts collection (previously referred to as customers)
- create account on user create, or use a session specific account
- implement Template.dynamic handling of AddressBook edit/add/grid
- changes to checkout address handling, fix accounts context
- update addressBook forms to autoform-v0.5.0 compatible (autoform not upgraded)
- change accounts/shop `email` from string to `emails` array
  - `emails: {'address': options.email, 'verified': true}`
- changed Shops, Accounts email to an array `emails`
- userAccountsDropdown icon for orders, profile
- user view of order history

TODO:

- merge session accounts to user account upon email confirmation (and email saved on cart)
- reduce / cleanup garbage sessions  (merge will help)
- session / accounts/ cart garbage collector
- account profile / settings
- on account creation / login  with password
    - if confirmed account creation email
      - create a account collection record with this userId
      - update all orders with matching email to match this userId
      - copy all order addresses into Accounts.profile.addressBook
      - copy all social / email info to Accounts.
      - users collection locked down, nothing exposed to client, used for authentication only
    - else
      - if there are more orders with this email
      - display on order view "Add order"

* moves user orders from cart/checkout folder to dashboard/orders
* add message for confirmation of email
* authenticated user can see all orders where userId in list view
* admin can see all in list view
* userAccountsDropdown icon for orders
* add cartId to Orders (instead of using cartId as orderId)

TODO:

- integrate the admin view of list into dashboard admin flow
- this is possibly a breaking change to the orders dashboard.
- remove sessionId from orders on logout

* updates to handling settings from registry (public, private)
* rename and move settingGeneral to shop/settings
* rename and move settingsAccounts to shop/accounts

**Multi-shop/vendor**

* shop account updates to prep multi-shop dashboard
* shopId added to cart.items (variants)

Issue updates:

- Strategic updates for issue #236
- Strategic updates for issue #327

* added settings.public to publish public settings to ReactionCore.xxx

Thanks to contributions and docs from @spencern

- Test coverage for product methods
- Fix, add coverage for core methods

* Upgrade to Meteor 1.0.4.2
* Implements `check` and `audit-argument-checks`
* Implements `browser-policy`
* Fixed footer layout pages loading.
* Updates CFS, removes FileStorage collection.
* **Contributions from @aldeed @prinzdezibel @spencern @boboci9 @evliu @ceh @gouthamve @KEFIRCHIK @epson21 @firstred
**
aaronjudd added a commit that referenced this issue Dec 3, 2015
**Cart**

*The goal is a reactive offline / online multisession-multishop-multidevice cart.*

- if there is neither a sessionCart nor a userCart, create a sessionCart
  * add sessionId to cart.sessions
  * auto insert/update userId if authenticated
  * observers to ensure cart always exists (ie after deletion)
- if sessionCart is a not a userCart return sessionCart
- if sessionCart just authenticated, add userId
- only return a user cart when authenticated
  * copy existing userCart(s) into sessionCart and remove userCart(s)
- if userCart just logged out, remove sessionId from userCart and create new sessionCart
- allow multiple sessionId per cart when userCart

TODO:

- realistically this may not be very solid way to handle sessions.
- It will need a close review in the future.
- there are some edge cases where multiple authentications
or merges to the same cart may destroy the cart
- Cart really should have mirrored client and server methods
to allow adding to cart while offline
- allow multiple sessionId per cart when userCart
- consider implementation of this as part of Accounts

Updates issues:

- Remove packages introduced in reactioncommerce/reaction-core#76
- Resolves #271
- Resolves #339
- Resolves #326
- Resolves #183
- Strategic updates for issue #16
- Strategic updates for issue #318
- Strategic updates for issue #76

**Checkout**

   * dashboard enable/disable guest checkout
   * change default step to guest checkout or create/sign-in
   * store email for order - after order completion if guest
   * not visible after destroyed session (ie: refresh)
   * updated checkout and progress bar to use
     cart value instead of session

TODO:

- validation on email guest entry
- add account creation instructions to an order completion email
- potentially add third prompt for account creation after email (or promos,etc)
- order emails!
- use workflow states to control checkout ui flow.

**Accounts**

  * use new Accounts collection (previously referred to as customers)
  * userAccountsDropdown icon for orders, profile

  TODO:

- account profile / settings
- on account creation / login  with password:

    if confirmed account creation email
      create a account collection record with this userId
      update all orders with matching email to match this userId
      copy all order addresses into Accounts.profile.addressBook
      copy all social / email info to Accounts.
      users collection locked down, nothing exposed to client, used for authentication only
    else
      if there are more orders with this email
      display on order view "Add order"

**Packages**

* added settings.public to publish public settings to ReactionCore.xxx

**Orders**

* moves user orders from cart/checkout folder to dashboard/orders
* add message for confirmation of email
* authenticated user can see all orders where userId in list view
* admin can see all in list view
* userAccountsDropdown icon for orders
* add cartId to Orders (instead of using cartId as orderId)

TODO:

- integrate the admin view of list into dashboard admin flow
- this is possibly a breaking change to the orders dashboard.

**Dashboard**

* updates to handling settings from registry
* rename and move settingGeneral to shop/settings
* rename and move settingsAccounts to shop/accounts

**Multi-shop/vendor**

* shop account updates to prep multi-shop dashboard
* shopId added to cart.items (variants)

Strategic updates for issue #236
Strategic updates for issue #327

**General**

* Fixed footer layout pages loading.
* Updates CFS, removes FileStorage collection.

Related issues @aldeed @prinzdezibel
aaronjudd added a commit that referenced this issue Dec 3, 2015
Thanks to contributions from @danielgindi `right to left internationalization` is now supported.

Take a look at the Russian or Hebrew languages to see RTL in action.

 - Resolves #176

Also contributions from  @firstred and @epson121 added Dutch and Croatian translations.

Thanks to contributions from @lovetostrike, the initial `reactioncommerce:reaction-social` package has been published, which gives basic social sharing functionality.

 - Resolves #61
 - Resolves #281

- if there is neither a sessionCart nor a userCart, create a sessionCart
  * add sessionId to cart.sessions
  * auto insert/update userId if authenticated
  * observers to ensure cart always exists (ie after deletion)
- if sessionCart is a not a userCart return sessionCart
- if sessionCart just authenticated, add userId
- only return a user cart when authenticated
  * copy existing userCart(s) into sessionCart and remove userCart(s)
- if userCart just logged out, remove sessionId from userCart and create new sessionCart
- allow multiple sessionId per cart when userCart

TODO:

- realistically this may not be very solid way to handle sessions.
- Session handling will need a **close review in the future**.
- potential edge cases where multiple session/auth/merge actions to a cart may destroy the cart
- cart really should have mirrored client and server methods and be tested offline

Issue updates:

- Remove packages introduced in reactioncommerce/reaction-core#76
- Resolves #271
- Resolves #339
- Resolves #326
- Resolves #183
- Strategic updates for issue #16
- Strategic updates for issue #318
- Strategic updates for issue #76

   * dashboard enable/disable **guest checkout**
   * change default step to **guest checkout** or create/sign-in
   * store email for order - **after order completion if guest**
   * not visible after destroyed session (ie: refresh)
   * updated checkout and progress bar to use
     cart value instead of session

TODO:

- validation on email guest entry
- add account creation instructions to an order completion email
- potentially add third prompt for account creation after email (or promos,etc)
- order emails!
- use workflow states to control checkout ui flow.

Thanks to initial contributions from @prinzdezibel!

- use new Accounts collection (previously referred to as customers)
- create account on user create, or use a session specific account
- implement Template.dynamic handling of AddressBook edit/add/grid
- changes to checkout address handling, fix accounts context
- update addressBook forms to autoform-v0.5.0 compatible (autoform not upgraded)
- change accounts/shop `email` from string to `emails` array
  - `emails: {'address': options.email, 'verified': true}`
- changed Shops, Accounts email to an array `emails`
- userAccountsDropdown icon for orders, profile
- user view of order history

TODO:

- merge session accounts to user account upon email confirmation (and email saved on cart)
- reduce / cleanup garbage sessions  (merge will help)
- session / accounts/ cart garbage collector
- account profile / settings
- on account creation / login  with password
    - if confirmed account creation email
      - create a account collection record with this userId
      - update all orders with matching email to match this userId
      - copy all order addresses into Accounts.profile.addressBook
      - copy all social / email info to Accounts.
      - users collection locked down, nothing exposed to client, used for authentication only
    - else
      - if there are more orders with this email
      - display on order view "Add order"

* moves user orders from cart/checkout folder to dashboard/orders
* add message for confirmation of email
* authenticated user can see all orders where userId in list view
* admin can see all in list view
* userAccountsDropdown icon for orders
* add cartId to Orders (instead of using cartId as orderId)

TODO:

- integrate the admin view of list into dashboard admin flow
- this is possibly a breaking change to the orders dashboard.
- remove sessionId from orders on logout

* updates to handling settings from registry (public, private)
* rename and move settingGeneral to shop/settings
* rename and move settingsAccounts to shop/accounts

**Multi-shop/vendor**

* shop account updates to prep multi-shop dashboard
* shopId added to cart.items (variants)

Issue updates:

- Strategic updates for issue #236
- Strategic updates for issue #327

* added settings.public to publish public settings to ReactionCore.xxx

Thanks to contributions and docs from @spencern

- Test coverage for product methods
- Fix, add coverage for core methods

* Upgrade to Meteor 1.0.4.2
* Implements `check` and `audit-argument-checks`
* Implements `browser-policy`
* Fixed footer layout pages loading.
* Updates CFS, removes FileStorage collection.
* **Contributions from @aldeed @prinzdezibel @spencern @boboci9 @evliu @ceh @gouthamve @KEFIRCHIK @epson21 @firstred
**
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

No branches or pull requests

3 participants