os-js / osjs-server

OS.js Server Module
https://manual.os-js.org
Other
19 stars 21 forks source link

Exposing session object signals for user login/logout #47

Closed miladhashemi closed 3 years ago

miladhashemi commented 3 years ago

Hi Anders, Last night you added two signal. In server If all of session(not only user) accessed, could provides more general solution. Because at moment it returns only user session.

andersevenrud commented 3 years ago

Sorry, but I don't understand what you're asking here.

I added two signals, both of which gets the user session data as a parameter.

mahsashadi commented 3 years ago

Hi. We are working at the same project.

The parameter in these signals is user profile stored in session (including id, name, usename, groups). It is the object which is returned from login function in authentication Service provider.


We need to have the whole session object which we stored some other data in it during login process

andersevenrud commented 3 years ago

We need to have the whole session object which we stored some other data in it during login process

Sorry, but I still don't understand this.

is the object which is returned from login function in authentication Service provider

Yes. Which is the "whole" object.

Can you give an example maybe ?

miladhashemi commented 3 years ago

Can you give an example maybe ? We store to general (key-value) in session. it's below:

session: {
user: {
id: 'someId',
name: 'someName',
username: 'someUsername',
group: 'admin',
projects: ['test', 'test1']
},
swiftAuth: {
...
}
}

at server side we need both keys, but in client we need just user key! Actually in server side we need req.session, note only req.session.user.

Moreover, we need to extend session object which is not accessable, since parameter is sent via Object.freeze() (in server).

andersevenrud commented 3 years ago

Okay. I can change this so that you get the entire session, not just the user.

Moreover, we need to extend session object which is not accessable, since parameter is sent via Object.freeze() (in server).

Mutating the session data outside the auth handler like this sounds like a really bad idea. Why do you need to do this ?

miladhashemi commented 3 years ago

I want to add project list to user session.

projects: ['test', 'test1']

To get project list, I need some data that we kept them in swiftAuth

andersevenrud commented 3 years ago

But why not add this to the user data in your authentication adapter ?

miladhashemi commented 3 years ago

Unfortunately some sensitive data like token kept in swiftAuth that we want hide it from client side, if we put swiftAuth in user then it accessible from client,right!?

andersevenrud commented 3 years ago

if we put swiftAuth in user then it accessible from client,right!?

Yes.

But what I was asking about was: I want to add project list to user session..

Example:

async login(req, res) {
  const result = await loginMyUser(req.body.username, req.body.password)
  req.session.swiftAuth = {}
  return result.profile // This should contain "project" etc.
}
andersevenrud commented 3 years ago

I've published @osjs/server@3.1.17 now. This makes it so you get a copy of the session data, not just session.user.

miladhashemi commented 3 years ago

Yes, there are two kind data. The data that could be sent to client(like project list that uses as mountpoint name), we sent them to client with user session, but second type is data mist be kept and access from server, like token, expiration date,...

For this purpose we decide to separate these two key(user-swiftAuth)

andersevenrud commented 3 years ago

Yes, I understand that, and that is totally fine.

My point is that you should not change this data outside of your authentication handler. Like what you mentioned by "I wat to add project list to user session". This all should ideally happen from within your adapter, not using the logged-in signal.

miladhashemi commented 3 years ago

I agree with you. At first we were getting project list in login method. But it had some shortages, first when we were implementing login method, we got project list(mountpoints) but actually these are two different operations. Secondly, we decide to decouple listing project list and login till a user decide uses only auth adapter, just install auth adapter to use openstack(tempAuth, keystone). And when users decide to get project list, they install swiftmountpoint ServiceProvider(server/client) to add mountpoints.

andersevenrud commented 3 years ago

Okay then :smile:

So this is resolved ? If so, please close the issue.

miladhashemi commented 3 years ago

We have one issue yet 😬, for getting project list we need update session. First we need modify session and add project list to user session in server login signal, then use user updated session in client service provider to create mount points. But you make session freeze, we can’t modify session 😕

andersevenrud commented 3 years ago

Yes, you can't modify the session outside of the authentication adapter. This is by design. Sessions are only used in the life-cycle of HTTP requests.

In this case when this signal fires, and you execute some asynchronous function inside of the callback, then you will be outside of this life-cycle. Your operation has no guarantee of completing before the client has gotten a response from this HTTP request.

NB: Signals are not handled asynchronously!

There are methods in place to help you out with your de-coupling issue:

const adapter = (core) => {
  const auther = core.make('my-login-service')

  return {
    async login(req, res) {
      const user = await auther.authenticate(req.body.username, req.body.password)
      const token = await auther.getToken(user)
      const result = {...user}

      if (core.has('my-other-service')) {
        const projects = await core.make('my-other-service').getProjects(user)
        result.projects = projects
      }

      req.session.token = token

      return result
  }
}

By the time the signal fires, your session is now complete. No need to mutate :smile:

andersevenrud commented 3 years ago

In addition to this life-cycle, look at how the signal is implemented: https://github.com/os-js/osjs-server/blob/5ab0caf8e564ed9063d66b74dfa3c62557bf1cef/src/auth.js#L134-L140

If you could somehow modify the session, it would not actually be saved and usable in future requests. Also, it's a copy, not a reference :nerd_face:

miladhashemi commented 3 years ago

Your operation has no guarantee of completing before the client has gotten a response from this HTTP request.

As I was describing scenario, I realized this issue :sweat_smile: , until your response I was thinking how I should solve this problem :grin:

Yesssss, You surprise me again. The has() method is extermlly helpful in these cases. :v:

andersevenrud commented 3 years ago

The has() method is extermlly helpful in these cases. :v:

Great! I think this will solve your issue :blush:

Let me know how it works out, and if it's all OK close this issue.

miladhashemi commented 3 years ago

If you could somehow modify the session, it would not actually be saved and usable in future requests. Also, it's a copy, not a reference

Only I was checked it where you send it call by value. Totally You remove call by reference for session. Just in HTTP request session could be modified :smiley: