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

add change manager #53

Open
smhaller opened this issue Jun 13, 2024 · 6 comments
Open

add change manager #53

smhaller opened this issue Jun 13, 2024 · 6 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@smhaller
Copy link
Owner

I had a quick look:

I think one should not enable this at the moment. It does not even work in one session (or better if you enable just the TrackChanges I had following behaviour:

  • Enable TrackChanges
  • Do some work
  • Accepted Changes

now if one reloads the Project you get again the field/box where you can Accept or Reject the changes.
Only if you Reject you can get rid again of the field/box.

In general I think following has to be done if one wants to enable the Track Changes and Comments Feature:

First one has to add all route in route.js

For example:

webRouter.get(
   '/project/:project_id/threads',
   AuthorizationMiddleware.blockRestrictedUserFromProject,
   AuthorizationMiddleware.ensureUserCanReadProject,
   ChatController.getThreads
)
webRouter.post(
   '/project/:project_id/thread/:thread_id/messages',
   AuthorizationMiddleware.blockRestrictedUserFromProject,
   AuthorizationMiddleware.ensureUserCanReadProject,
   ChatController.sendComment
)
webRouter.post(
   '/project/:project_id/thread/:thread_id/resolve',
   AuthorizationMiddleware.blockRestrictedUserFromProject,
   AuthorizationMiddleware.ensureUserCanReadProject,
   ChatController.resolveThread
)

Missing routes I have seen so far:
/project/:project_id/changes/accept
/project/:project_id/threads
/project/:project_id/thread/:thread_id/messages
/project/:project_id/thread/:thread_id/resolve
/project/:project_id/doc/:thread_id/changes/accept
/project/:project_id/ranges
/project/:project_id/track_changes
/event/rp-comment-resolve

Second one has to adapt at least the ChatController.js

and add the missing code between the ChatController and the ChatApiHandler.js
Be aware the following Code is not working yet (the reason why I post it in a comment and not push it in some branch in the repo):

 sendComment(req, res, next) {
     const { project_id: projectId } = req.params
     const { thread_id: threadId } = req.params
     const { content, client_id: clientId } = req.body
     const userId = SessionManager.getLoggedInUserId(req.session)
     if (userId == null) {
       const err = new Error('no logged-in user')
       return next(err)
     }
     return ChatApiHandler.sendComment(
       projectId,
       threadId,
       userId,
       content,
       function (err, message) {
         if (err != null) {
           return next(err)
         }
         return UserInfoManager.getPersonalInfo(
           message.user_id,
           function (err, user) {
             if (err != null) {
               return next(err)
             }
             message.user = UserInfoController.formatPersonalInfo(user)
             message.clientId = clientId
             EditorRealTimeController.emitToRoom(
               projectId,
               'rp-new-comment',
               message
             )
             return res.sendStatus(204)
           }
        )
      }
   )
},

and maybe some more

Conclusion

It would be really cool if someone could have a look into this - but I also think this is a bit of work (but I think it is possible because the API/backend code seems to be there

@smhaller smhaller added enhancement New feature or request help wanted Extra attention is needed labels Jun 13, 2024
@yu-i-i
Copy link

yu-i-i commented Jun 19, 2024

Here, https://github.com/yu-i-i/overleaf-cep/, you can find a patched version of Overleaf with LDAP authentication and a change tracking feature. For LDAP, I used your code but modified it to use passport-ldapauth. The environment variables for LDAP configuration now correspond to those in Overleaf Pro, with one exception: instead of OVERLEAF_LDAP_NAME_ATT, I use OVERLEAF_LDAP_FIRST_NAME_ATT. Additionally, I rewrote the LDAP contacts as a module without changing the original code.

The change tracking feature works mostly as expected. There is one minor glitch (see comments in the code).

I’m not a programmer, just an Overleaf user, and I don’t intend to develop this code further. However, if someone else wants to continue, that would be great. For example, adding features like templates or Dropbox integration (or even Nextcloud).

@smhaller
Copy link
Owner Author

smhaller commented Jun 24, 2024

Thx @yu-i-i !
I adopted your changes and created a development branch (see 5.0.6 branch) in this repository. I still have to test the OAuth part but all in all it looks good.

Notes : (to myself)

  • I had to pull 4 files from the overleaf git repository (because those in the overleaf Docker Images were too old)
  • I have to think about @yu-i-i solution of creating modules: This looks also nice (but for me they didn't load with the Sharelatex/Overleaf Docker Image - I need a bit time to look at this).
  • If we decide to switch to the modules way of doing things we have to cleanup our repo and add 3 modules (ldap-auth, oauth2-auth, and track-changes). We should anyway keep the script solution for files like e.g. web/app/src/Features...

Lets see that the 5.0.6 branch will close Issue #50, #53 and Pull Request #51

@smhaller
Copy link
Owner Author

@yzx9 do you have time to look at the 5.0.6 branch ?

@yzx9
Copy link
Collaborator

yzx9 commented Jun 28, 2024

@smhaller Apologies for the delay, I reviewed your new branch and comments. I didn't find breaking change, but I suggest splitting the changes into multiple PRs to simplify the review process. We can bump to 5.0.6 first and then add the other changes.

@yzx9
Copy link
Collaborator

yzx9 commented Jun 29, 2024

> // If no LDAP Server is in use and no local db login then we can redirect the login
> // and just use OAUTH
> if ( (typeof process.env.LDAP_SERVER === typeof undefined) && (process.env.ALLOW_EMAIL_LOGIN === 'false') && (process.env.OAUTH2_ENABLED === 'true') ) {
> webRouter.get('/login', function (req, res, next) { res.redirect('/oauth/redirect') })
> } else {
> webRouter.get('/login', UserPagesController.loginPage)
> }
>

Disabling the login page may result in an infinite redirection loop. If the OAuth2 login fails, the user is redirected from oauth/callback to the /login page, which then redirects back to /oauth/redirect, causing the user to fall into oauth/callback again, creating a continuous cycle.

To address this issue, we need to add an error page that displays the error message and provides a link for the user to try the OAuth2 login again.

@yu-i-i
Copy link

yu-i-i commented Jul 24, 2024

@smhaller

Here we found out that in images up to version 5.0.7 the old API is used. Starting from version 5.1.0, the new API is used. In Overleaf git code the new API is used already about half a year. In order to make comments feature to work properly in 5.0.6, the following changes have to be done:

In TrackChangesRouter.js:

    webRouter.post(
      '/project/:project_id/thread/:thread_id/resolve',
      AuthorizationMiddleware.blockRestrictedUserFromProject,
      AuthorizationMiddleware.ensureUserCanReadProject,
      TrackChangesController.resolveThread
    )
    webRouter.post(
      '/project/:project_id/thread/:thread_id/reopen',
      AuthorizationMiddleware.blockRestrictedUserFromProject,
      AuthorizationMiddleware.ensureUserCanReadProject,
      TrackChangesController.reopenThread
    )

In TrackChangesController.js:

  resolveThread(req, res, next) {
    const { project_id, thread_id } = req.params
    const user_id = SessionManager.getLoggedInUserId(req.session)
    if (user_id == null) {
      const err = new Error('no logged-in user')
      return next(err)
    }
    return ChatApiHandler.resolveThread(
      project_id,
      thread_id,
      user_id,
      function (err, message) {
        if (err != null) {
          return next(err)
        }
        return UserInfoManager.getPersonalInfo(
          user_id,
          function (err, user) {
            if (err != null) {
              return next(err)
            }
            EditorRealTimeController.emitToRoom(
              project_id,
              'resolve-thread',
              thread_id,
              user
            )
            return res.sendStatus(204)
          }
        )
      }
    )
  },
  reopenThread(req, res, next) {
    const { project_id, thread_id } = req.params
    const user_id = SessionManager.getLoggedInUserId(req.session)
    if (user_id == null) {
      const err = new Error('no logged-in user')
      return next(err)
    }
    return ChatApiHandler.reopenThread(
      project_id,
      thread_id,
      function (err, message) {
        if (err != null) {
          return next(err)
        }
        EditorRealTimeController.emitToRoom(
          project_id,
          'reopen-thread',
          thread_id
        )
        return res.sendStatus(204)
      }
    )
  },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants