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

User auth/Role-based Access Control #146

Open
htoraby opened this issue Sep 2, 2024 · 26 comments
Open

User auth/Role-based Access Control #146

htoraby opened this issue Sep 2, 2024 · 26 comments

Comments

@htoraby
Copy link

htoraby commented Sep 2, 2024

Hi, I see that the user access control dose not work properly on bellow cases:

  1. In Display Viewer on Choose Screen. The list should be updated based on user access screen list
  2. In Alarm Viewer the list of Group1 and all related alarms shown on screen is not based on Group1 access on user role.
  3. In Event Viewer the events that are listed should be based on user access role. Also the for the filter icon
  4. The Beep and Beep icon should be based on each user space.
  5. In Tabular Viewer the list of Substation should be based on user role. Actually if a user that do not access to the desired Group1 nothing will appear but the Substation list all.

Anyway I tested bellow modification and it is work OK for items 2, 5 above but this do not affect on other items of above

  • on the src/server_realtime_auth/index.js on bellow of case
      case opc.ServiceCode.Extended_RequestUniqueAttributeValues: // list unique attribute values
.......
line 2123:
          let Results = []
          await results.map((node) => {
            if (AUTHENTICATION && !userRights?.isAdmin && userRights?.group1List.length > 0) {
              if ( !userRights.group1List.includes(node.group1) )
                {
                  // Access to data denied! 
                  return node;
                }
            }
            let Value = {
              Value: {
                Type: opc.DataType.String,
                Body: node.group1,
                Count: node.count,
              },
            }
            Results.push(Value)
            return node
          })

@riclolsen
Copy link
Owner

Hi Hojat!
Yes, all of those are missing features yet to be implemented. There are many things to implement at UI level.
I am focused on protocols development right now, but will try to address the list for the next version.
Thanks very much for the contribution.
Ricardo

@htoraby
Copy link
Author

htoraby commented Sep 2, 2024

Hi Ricardo,
I am very interest on your project, what I see on your work is a wonderful.
I hope to see your updates soon, I am thinking if possible to have secured multi-user, multi-area SCADA based on this.
Best Regards,

@riclolsen
Copy link
Owner

Thanks for the kind words Hojat, updates will sure come soon!
This project is very ambitious, it was designed to have as few limitations as possible in theory, features are to be implemented gradually.
Best regards!

@htoraby
Copy link
Author

htoraby commented Sep 2, 2024

Hello Ricardo, Is it possible to share me the admin password of your demo server http://150.230.171.172/ ? Actually I would like to see about detail how data configured for each specific points.
Actually I installed from source on the RockyLinux distribution with new data, all things work but Replay or follow measure dose not work.
Thanks

@riclolsen
Copy link
Owner

riclolsen commented Sep 3, 2024

This demo is a pretty old version.
I will check the errors, it can be something related to postgresql installation or initialization.
There are some tag parameters like protocolDestinations for server protocol IEC104 that are not included in the UI. This can only be adjusted in Excel or directly on mongodb.

@htoraby
Copy link
Author

htoraby commented Sep 3, 2024

Hi Ricardo, Yes the problem was based on socket port number of postgresql in installation. It is work now, but Replyy dose not work for before time of now < 3hours. My mean is that If I change the ruler to the 1 hour before it jumped to last, but if I click on more than 3 hour before it replay data on that time.

@riclolsen
Copy link
Owner

I have found a missing package problem in the installer script on Rocky Linux. It is now fixed.

@riclolsen
Copy link
Owner

Items 2, 3 and 5 are solved in recent commit.
Item 1 persists but display contents are limited to authorized group1 list of current user/role.

@htoraby
Copy link
Author

htoraby commented Sep 4, 2024

Hi, the display contents are limited it is OK, but the list of choose screen should be based on limited access to items "Can Access - Display List" on user role. I think the the file "svg/screen_list.js" should be updated.
For this issue I suggest to have two fields on database one for SVG screen file path/name and another to the display text name for Choose screen items list shown on drop down list. Then got from server the list that include the file path and name and prepare the optionhtml variable content.

@htoraby
Copy link
Author

htoraby commented Sep 4, 2024

Items 2, 3 and 5 are solved in recent commit. Item 1 persists but display contents are limited to authorized group1 list of current user/role.

For item 3, if a user click on "Erase All Event F2" so all event on the system that do not have access on them also will be erased. It is problem, could you check.
For item 2, if a user click on "Acknowledge all events! F8" so all event on the system that do not have access on them also acknowledged. It is problem, could you check.

@htoraby
Copy link
Author

htoraby commented Sep 4, 2024

Items 2, 3 and 5 are solved in recent commit. Item 1 persists but display contents are limited to authorized group1 list of current user/role.

For item 3, if a user click on "Erase All Event F2" so all event on the system that do not have access on them also will be erased. It is problem, could you check. For item 2, if a user click on "Acknowledge all events! F8" so all event on the system that do not have access on them also acknowledged. It is problem, could you check.

Above problem resolved after checking the last commit Now Item 1 and 4 remained

@htoraby
Copy link
Author

htoraby commented Sep 4, 2024

About Item 4, I think BEEP_POINTKEY should move to "users" database scheme, what you think?

@riclolsen
Copy link
Owner

I think we should preserve the global beep for the server to generate sound and a user beep that only sounds for the current user in the Alarms Viewer. It will require some work. Will check.

@htoraby
Copy link
Author

htoraby commented Sep 6, 2024

The Alarm Beep should be based on the same user on each browser and for each user based on his access to the Group1 as like as what is in the event list or alarm list now. In other words, beep activation/deactivation and beep alarm generation must be done separately for each user. For example, in Group name SS1, which user name "user1" has access to, if an alarm appears, only "user1" will hear the beep on his browser, and if he disables it, it will be disabled on all browsers that "user1" has opened, and this issue It does not happen to "user2" who does not have access to Group name SS1. So the beep alarm and its activation and deactivation should be based on the logged-in user and based on their access list.
If the global beep preserve it should not cause to play beep if user do not access to the alarm that occurred and it could only play beep for admin user for example for the server as you said.

@riclolsen
Copy link
Owner

Ok, now alarm beep on browser will follow user rights to group1 list.
Global beep is only for the alarm_beep process on the server.

@htoraby
Copy link
Author

htoraby commented Sep 6, 2024

The Bell sign now appears on screen of each user space alarm, great work!
There is a problem in play sound of alarm (tabular.html) yet, please checked if bellow suggestion is OK

    // alarm beep sound
    if (WebSAGE.isAlarmsViewer())
      document.getElementById('btSound').style.display='';
    document.getElementById('btSound').addEventListener("click", function () {      
        if (document.getElementById('btSound').value === '🔊'){
          document.getElementById('btSound').value = '🔈';
          clearInterval(window.sndIntervalHandle);
        } else {
          document.getElementById('nonCriticalSound').play();
          document.getElementById('btSound').value = '🔊';
          window.sndIntervalHandle = setInterval( function(){        
//change this line to bellow line:  if (window.WebSAGE.getValue(BEEP_POINTKEY))
          if ( ALARMBEEP ) // <-- this comment
            switch (ALARMBEEP_TYPE) {
              default:
                document.getElementById('nonCriticalSound').play();
                break;
              case 2:
                document.getElementById('criticalSound').play();
                break;                 
            }
          }, 1500);
        }
      });
    }

@riclolsen
Copy link
Owner

Sure, that fixed it.

@htoraby
Copy link
Author

htoraby commented Sep 7, 2024

I see just Item1 is remained, did you check the suggestion #146 (comment) ?

@riclolsen
Copy link
Owner

riclolsen commented Sep 7, 2024

Ok. The list will be built by allowed displays when restricted. Otherwise will load from screen_list.js file.

@htoraby
Copy link
Author

htoraby commented Sep 8, 2024

In src/htdocs-login/login.html there is a bug for display after your fix for bellow:

      <div class="blocks">
        <a target="_blank" href="../display.html?SVGFILE=../svg/kaw2.svg">
          <img height="40" src="images/tela.png" /><br />Display Viewer
        </a>
      </div>

Please check about that, actually I changed that to bellow with the empty screen image:

      <div class="blocks">
        <a target="_blank" href="../display.html?SVGFILE=../svg/screen.svg'">
          <img height="40" src="images/tela.png" /><br />Display Viewer
        </a>
      </div>

Then change the screen_list.js as bellow:

optionhtml = "\
 <option id='SELTELA_OPC1' selected disabled='disabled'>Choose a screen ...</option>\
 <optgroup label='Substation'>\
</optgroup>\
";

What is your opinion?

@riclolsen
Copy link
Owner

I changed it to not open any default display.

@htoraby
Copy link
Author

htoraby commented Sep 12, 2024

Hi, in the Admin form if a in User Role we have a enabled "Change Password", if user login with this privileged; there is no place that able to change its own password. To change the its own password, need to enter previous password with two repeated new password.

@Zarvhos
Copy link

Zarvhos commented Sep 12, 2024

Hi, in the Admin form if a in User Role we have a enabled "Change Password", if user login with this privileged; there is no place that able to change its own password. To change the its own password, need to enter previous password with two repeated new password.

Hi!

You're right, this seems to be a small issue with the Admin/Server_realtime_auth system. There’s definitely room for refactoring here. For example, the system could have automatic redirection on certain events (e.g., when the JWT token expires, instead of showing a JSON message). The admin frontend could also benefit from some UX improvements for forms, user validation, and visual feedback (e.g., loading indicators, user responses).

Another concern is the lack of a check for duplicate users when an admin creates an account, which could lead to two accounts sharing the same username and cause problems. There's also the limitation where an admin cannot update other admin roles/users, which might be better addressed by allowing admins to modify admin users/permissions, or perhaps creating a super admin role with full permissions.

Instead of flooding this issue with everything, we could create separate issues for each of these problems. As for the admin UI and server_realtime/_auth improvements, I can suggest some features to help @riclolsen lighten the load.

Let me know what you think!

Best,
Zarvhos.

@riclolsen
Copy link
Owner

Now the user can change its own password in the login form (after signin).
Admins still can change rights and force password for any user.

@riclolsen
Copy link
Owner

The database has a restriction to avoid duplication of user accounts (same username).
There is no check or good feedback in the app indeed however.

@htoraby
Copy link
Author

htoraby commented Sep 12, 2024

It seems, it is forgot to add the Invoke/auth/changePassword into server_realtime_auth\app\routes\auth.routes.js

app.post(accessPoint + 'auth/changePassword', controller.changePassword)

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