Rule: Door Access Control with RFID (help with rule / var definition / improvement)

Hi.

I have three garden door and for this i would like use RFID Coins to allow the access. the fully sketch are attached. The hardware construction works seperate, but i have a problem with the rule. I think it is easy for a programmer.

If u hold the coin on the RFID Reader, the PI change the VI_Sec_AC_Read_String (example: 124345678901,Max,1) → (UID, UserTag, DoorID)

The rule check if a VI_Sec_AC_UserN with the same UID and UserTag in the Group Grp_Sec_AC_User and it VI_Sec_AC_UserN String contains the requesting DoorID.

The Problem in the rule is how i define the variable DoorID so that i can used it.

Error from Log:

   1. The method or field Door1 is undefined; line 37, column 1765, length 5

   2. The method or field Door2 is undefined; line 37, column 1786, length 5

   3. The method or field Door3 is undefined; line 37, column 1807, length 5

Perhaps you still have fundamental suggestions for improvement.

  • Platform information:

    • Hardware: Synology 916+
    • openHAB version: OH 3.1.0 (Docker)
  • Items:

    • Grp_Sec_AC_User (Type: Group)
    • VI_Sec_AC_User1 (Type: String) (MemberOf: Grp_Sec_AC_User )
    • VI_Sec_AC_UserN (Type: String) (MemberOf: Grp_Sec_AC_User )
    • VI_Sec_AC_Read_String (Type: String)
  • Rule:

/* User String: 0=(UserID), 1=(UserName),2-4=(DoorID) */

val telegramActionSec = getActions("telegram","telegram:telegramBot:TelegramBotSec")

// ##################################################################

val UserID = VI_Sec_AC_Read_String.state.toString.split(",").get(0)
val UserName = VI_Sec_AC_Read_String.state.toString.split(",").get(1)
val DoorID = VI_Sec_AC_Read_String.state.toString.split(",").get(2)  
// ################################################################## 

switch (DoorID) {
// ##### Garden Door (Main) #####
  case "1": {
      Grp_Sec_AC_User.members.filter[state.toString.contains(UserID) && state.toString.contains(UserName)].forEach[User|
        var CountUserString = User.state.toString.split(",").length
        logInfo("System","COUNT: " + CountUserString)  
        if (CountUserString == 3) {
          val Door1 = User.state.toString.split(",").get(2)
          val Door2 = "0"
          val Door3 = "0"
          logInfo("System","Case 1.3")     
        } 
        if (CountUserString == 4){
          val Door1 = User.state.toString.split(",").get(2)                                                                                                       
          val Door2 = User.state.toString.split(",").get(3)
          val Door3 = "0"
          logInfo("System","Case 1.4")    
        } 
        if (CountUserString == 5){
          val Door1 = User.state.toString.split(",").get(2)                                                                                                       
          val Door2 = User.state.toString.split(",").get(3)            
          val Door3 = User.state.toString.split(",").get(4)
          logInfo("System","Case 1.5 - " + Door1)   
        }
 
        if ((DoorID == Door1) || (DoorID == Door2) || (DoorID == Door3)) {
          logInfo("System","Sicherheit - Information: Garten Haupteingang wird geöffnet durch User: " + User.state.toString.split(",").get(1))  
        } else {
          logInfo("System","Sicherheit - Warnung: ACCESS DENIED - Hof Tür - UID: " + User.state.toString.split(",").get(0) + " - UserName: " +  User.state.toString.split(",").get(1))    
        }
      ]
    }  
// ##### Garage Door #####
  case "2": {
      Grp_Sec_AC_User.members.filter[state.toString.contains(UserID) && state.toString.contains(UserName) && (state.toString.split(",").get(2)==DoorID || state.toString.split(",").get(3)==DoorID)].forEach[User|
        logInfo("System","Sicherheit - Information: Garagen Tür wird geöffnet durch User: " + User.state.toString.split(",").get(1))
      ]
    } 
// ##### Garden Door (Side) #####
  case "3": {
      Grp_Sec_AC_User.members.filter[state.toString.contains(UserID) && state.toString.contains(UserName) && (state.toString.split(",").get(2)==DoorID || state.toString.split(",").get(3)==DoorID)].forEach[User|
        logInfo("System","Sicherheit - Information: Garten Nebeneingang wird geöffnet durch User: " + User.state.toString.split(",").get(1))
      ] 
    }   
// ##### Default (wrong DoorID) #####
  default: {
      logInfo("System","Sicherheit - Warnung: wrong Door ID - ACCESS DENIED - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)    
    }
}

Door1, Door2 etc. are defined only within the if() code block, defined within the braces { }
After the }, they are discarded.

Here is a way to populate a variable that will be available after the { } block

        var Door1 = "0"
        if (CountUserString == 3) {
          Door1 = User.state.toString.split(",").get(2)
        }

Note that it uses var not val, so that you can alter it later.

In your rule, note that switch() and case() use { } as well.

Thx, in the past I’ve had defined this before the switch () and that doesn’t work for the same reason

        var Door1 = "0"
        var Door2 = "0"
        var Door3 = "0"
/* User String: 0=(UserID), 1=(UserName),2-4=(DoorID) */
// ##################################################################
val telegramActionSec = getActions("telegram","telegram:telegramBot:TelegramBotSec")
// ##################################################################
val UserID = VI_Sec_AC_Read_String.state.toString.split(",").get(0)
val UserName = VI_Sec_AC_Read_String.state.toString.split(",").get(1)
val DoorID = VI_Sec_AC_Read_String.state.toString.split(",").get(2)  
// ################################################################## 

switch (DoorID) {
// ##### Garden Door (Main) #####
  case "1": {
      Grp_Sec_AC_User.members.filter[state.toString.contains(UserID) && state.toString.contains(UserName)].forEach[User|
        var CountUserString = User.state.toString.split(",").length
        var Door1 = "0"
        var Door2 = "0"
        var Door3 = "0"
         
        if (CountUserString == 5){
          Door1 = User.state.toString.split(",").get(2)                                                                                                       
          Door2 = User.state.toString.split(",").get(3)            
          Door3 = User.state.toString.split(",").get(4)  
        } else {
          logInfo("System","Sicherheit - Warnung: wrong String - ACCESS DENIED - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)  
        }
         
        if ((DoorID == Door1) || (DoorID == Door2) || (DoorID == Door3)) {
          logInfo("System","Sicherheit - Information: Garten Haupteingang wird geöffnet durch User: " + User.state.toString.split(",").get(1))  
        } else {
          logInfo("System","Sicherheit - Warnung: ACCESS DENIED - Garten Haupteingang - UID: " + User.state.toString.split(",").get(0) + " - UserName: " +  User.state.toString.split(",").get(1))    
        }
      ]
    }  
// ##### Garage Door #####
  case "2": {
      Grp_Sec_AC_User.members.filter[state.toString.contains(UserID) && state.toString.contains(UserName)].forEach[User|
        var CountUserString = User.state.toString.split(",").length
        var Door1 = "0"
        var Door2 = "0"
        var Door3 = "0"
          
        if (CountUserString == 5){
          Door1 = User.state.toString.split(",").get(2)                                                                                                       
          Door2 = User.state.toString.split(",").get(3)            
          Door3 = User.state.toString.split(",").get(4)  
        } else {
          logInfo("System","Sicherheit - Warnung: wrong String - ACCESS DENIED - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)  
        }

        if ((DoorID == Door1) || (DoorID == Door2) || (DoorID == Door3)) {
          logInfo("System","Sicherheit - Information: Garagen Tür wird geöffnet durch User: " + User.state.toString.split(",").get(1)) 
        } else {
          logInfo("System","Sicherheit - Warnung: ACCESS DENIED - Garagen Tür - UID: " + User.state.toString.split(",").get(0) + " - UserName: " +  User.state.toString.split(",").get(1))    
        }
      ]
    }    
// ##### Garden Door (Side) #####
  case "3": {
      Grp_Sec_AC_User.members.filter[state.toString.contains(UserID) && state.toString.contains(UserName)].forEach[User|
        var CountUserString = User.state.toString.split(",").length  
        var Door1 = "0"
        var Door2 = "0"
        var Door3 = "0"
         
        if (CountUserString == 5){
          Door1 = User.state.toString.split(",").get(2)                                                                                                       
          Door2 = User.state.toString.split(",").get(3)            
          Door3 = User.state.toString.split(",").get(4)   
        } else {
          logInfo("System","Sicherheit - Warnung: wrong String - ACCESS DENIED - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)  
        }
 
        if ((DoorID == Door1) || (DoorID == Door2) || (DoorID == Door3)) {
          logInfo("System","Sicherheit - Information: Garten Nebeneingang wird geöffnet durch User: " + User.state.toString.split(",").get(1))
        } else {
          logInfo("System","Sicherheit - Warnung: ACCESS DENIED - Garten Nebeneingang - UID: " + User.state.toString.split(",").get(0) + " - UserName: " +  User.state.toString.split(",").get(1))
        }
      ]
    }    
// ##### Default (wrong DoorID) #####
  default: {
      logInfo("System","Sicherheit - Warnung: wrong Door ID - ACCESS DENIED - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)    
    }
}

And may we see that reason? Maybe with a bit of context? You’ve got other logging to help follow the path, currently secret.

that was in the past, now it works. :wink:

Okay.
You could do your var Doors before the switch() just once, save code.

   1. Cannot refer to the non-final variable Door1 inside a lambda expression; line 23, column 1216, length 5

   2. Cannot refer to the non-final variable Door2 inside a lambda expression; line 24, column 1377, length 5

   3. Cannot refer to the non-final variable Door3 inside a lambda expression; line 25, column 1447, length 5

   4. Cannot refer to the non-final variable Door1 inside a lambda expression; line 30, column 1709, length 5

   5. Cannot refer to the non-final variable Door2 inside a lambda expression; line 30, column 1730, length 5

   6. Cannot refer to the non-final variable Door3 inside a lambda expression; line 30, column 1751, length 5

   7. Cannot refer to the non-final variable Door1 inside a lambda expression; line 43, column 2448, length 5

   8. Cannot refer to the non-final variable Door2 inside a lambda expression; line 44, column 2609, length 5

   9. Cannot refer to the non-final variable Door3 inside a lambda expression; line 45, column 2679, length 5

   10. Cannot refer to the non-final variable Door1 inside a lambda expression; line 50, column 2941, length 5

   11. Cannot refer to the non-final variable Door2 inside a lambda expression; line 50, column 2962, length 5

   12. Cannot refer to the non-final variable Door3 inside a lambda expression; line 50, column 2983, length 5

   13. Cannot refer to the non-final variable Door1 inside a lambda expression; line 63, column 3674, length 5

   14. Cannot refer to the non-final variable Door2 inside a lambda expression; line 64, column 3835, length 5

   15. Cannot refer to the non-final variable Door3 inside a lambda expression; line 65, column 3905, length 5

   16. Cannot refer to the non-final variable Door1 inside a lambda expression; line 70, column 4168, length 5

   17. Cannot refer to the non-final variable Door2 inside a lambda expression; line 70, column 4189, length 5

   18. Cannot refer to the non-final variable Door3 inside a lambda expression; line 70, column 4210, length 5
/* User String: 0=(UserID), 1=(UserName),2-4=(DoorID) */
// ##################################################################
val telegramActionSec = getActions("telegram","telegram:telegramBot:TelegramBotSec")
// ##################################################################
val UserID = VI_Sec_AC_Read_String.state.toString.split(",").get(0)
val UserName = VI_Sec_AC_Read_String.state.toString.split(",").get(1)
val DoorID = VI_Sec_AC_Read_String.state.toString.split(",").get(2)  
// ################################################################## 
  
if(  Grp_Sec_AC_User.members.filter[state.toString.contains(UserID) && state.toString.contains(UserName)].size == 0) {
  logInfo("System","Sicherheit - Warnung: User not found - ACCESS DENIED - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)    
} else {
  var Door1 = "0"
  var Door2 = "0"
  var Door3 = "0"   
  switch (DoorID) { 
  // ##### Garden Door (Main) #####
    case "1": {
        Grp_Sec_AC_User.members.filter[state.toString.contains(UserID) && state.toString.contains(UserName)].forEach[User|
          var CountUserString = User.state.toString.split(",").length

          if (CountUserString == 5){
            Door1 = User.state.toString.split(",").get(2)                                                                                                       
            Door2 = User.state.toString.split(",").get(3)            
            Door3 = User.state.toString.split(",").get(4)  
          } else {
            logInfo("System","Sicherheit - Warnung: wrong String - ACCESS DENIED - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)  
          }

          if ((DoorID == Door1) || (DoorID == Door2) || (DoorID == Door3)) {
            logInfo("System","Sicherheit - Information: Garten Haupteingang wird geöffnet durch User: " + User.state.toString.split(",").get(1))  
          } else {
            logInfo("System","Sicherheit - Warnung: ACCESS DENIED - Garten Haupteingang - UID: " + User.state.toString.split(",").get(0) + " - UserName: " +  User.state.toString.split(",").get(1))    
          }
        ]
      }  
  // ##### Garage Door #####
    case "2": {
        Grp_Sec_AC_User.members.filter[state.toString.contains(UserID) && state.toString.contains(UserName)].forEach[User|
          var CountUserString = User.state.toString.split(",").length

          if (CountUserString == 5){
            Door1 = User.state.toString.split(",").get(2)                                                                                                       
            Door2 = User.state.toString.split(",").get(3)            
            Door3 = User.state.toString.split(",").get(4)  
          } else {
            logInfo("System","Sicherheit - Warnung: wrong String - ACCESS DENIED - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)  
          }

          if ((DoorID == Door1) || (DoorID == Door2) || (DoorID == Door3)) {
            logInfo("System","Sicherheit - Information: Garagen Tür wird geöffnet durch User: " + User.state.toString.split(",").get(1)) 
          } else {
            logInfo("System","Sicherheit - Warnung: ACCESS DENIED - Garagen Tür - UID: " + User.state.toString.split(",").get(0) + " - UserName: " +  User.state.toString.split(",").get(1))    
          }
        ]
      }    
  // ##### Garden Door (Side) #####
    case "3": {
        Grp_Sec_AC_User.members.filter[state.toString.contains(UserID) && state.toString.contains(UserName)].forEach[User|
          var CountUserString = User.state.toString.split(",").length  

          if (CountUserString == 5){
            Door1 = User.state.toString.split(",").get(2)                                                                                                       
            Door2 = User.state.toString.split(",").get(3)            
            Door3 = User.state.toString.split(",").get(4)   
          } else {
            logInfo("System","Sicherheit - Warnung: wrong String - ACCESS DENIED - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)  
          }

          if ((DoorID == Door1) || (DoorID == Door2) || (DoorID == Door3)) {
            logInfo("System","Sicherheit - Information: Garten Nebeneingang wird geöffnet durch User: " + User.state.toString.split(",").get(1))
          } else {
            logInfo("System","Sicherheit - Warnung: ACCESS DENIED - Garten Nebeneingang - UID: " + User.state.toString.split(",").get(0) + " - UserName: " +  User.state.toString.split(",").get(1))
          }
        ]
      }    
  // ##### Default (wrong DoorID) #####
    default: {
        logInfo("System","Sicherheit - Warnung: wrong Door ID - ACCESS DENIED - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)    
      }
  }
}

Show the full rule including the triggers and conditions. Click on the Code tab in the upper right. We are missing important context without that.

If I understand the code correctly you have one Item per member and each Item contains a comma separated list of the doors they are authorized to access. If this is correct, the above code is really over complicated and duplicative.

The steps that need to be performed are as follows:

  • figure out which user and which door is requesting an unlock, which you have done well
  • determine if that user is authorized for that door
  • if authorized unlock the door

All of these three steps can be performed generically. You don’t need to have a switch statement with three identical case statement whose only difference is the logging statements. There is a saying in programming: DRY - Do not Repeat Yourself.

So your whole rule could be:

/* User String: 0=(UserID), 1=(UserName),2-4=(DoorID) */
// ##################################################################
val telegramActionSec = getActions("telegram","telegram:telegramBot:TelegramBotSec")
// ##################################################################
val UserID = VI_Sec_AC_Read_String.state.toString.split(",").get(0)
val UserName = VI_Sec_AC_Read_String.state.toString.split(",").get(1)
val DoorID = VI_Sec_AC_Read_String.state.toString.split(",").get(2)  
// ################################################################## 

// Why look through all the records when all you care about is the one record for UserID?
val authDoors = Grp_Sec_AC_User.members.findFirst[state.toString.contains(UserID) && state.toString.contains(UserName)]

// Fail fast and exist leads to less complicated code
if(authDoors === null ) {
  logInfo("System","Sicherheit - Warnung: User not found - ACCESS DENIED - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)
  return;
}

if(authDoors.state.toString.contains(DoorID)){
  // allowed, unlock the door
}
else {
  // not allowed, generate an alert
}

The above code doesn’t care about how many doors nor how many users you have. You can add/remove either with out ever needing to modify the rule. That’s the ultimate goal because code you don’t have to touch is code you are unlikely to break later on. Use the Item name, Item variable, or add a Map to get a nice friendly name for your log statements.

But to solve the specific problem that you’ve encountered with your code as is relatively simple. Move the definitiosn of the Door variable to be inside the forEach loop (right after var CountUserString = User...). But you will basically have an identical block of code for each door repeated almost verbatim.

From a security perspective, I’d consider not using an Item to contain the mapping for which users are authorized on which doors. Anyone that can access your OH instance’s REST API will be able to change those Items and give themselves permissions. The risk is admittedly low but using an Item like that is little better than leaving a spare key under the welcome mat.

thx for the helpful suggestion. i’ve change the code.

do u have a better idea to store the authorized users than into a item?

/* User String: 0=(UserID), 1=(UserName),2-4=(DoorID) */
// ##################################################################
val telegramActionSec = getActions("telegram","telegram:telegramBot:TelegramBotSec")
// ##################################################################
val UserID = VI_Sec_AC_Read_String.state.toString.split(",").get(0)
val UserName = VI_Sec_AC_Read_String.state.toString.split(",").get(1)
val DoorID = VI_Sec_AC_Read_String.state.toString.split(",").get(2)
// ################################################################## 
val authDoors = Grp_Sec_AC_User.members.findFirst[state.toString.contains(UserID) && state.toString.contains(UserName)]

if(authDoors === null ) {
  logInfo("System","Security - Warning: ACCESS DENIED - User not found - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)
  telegramActionSec.sendTelegram("Security - Warning: \nACCESS DENIED - User not found \n- UID: " + UserID + " \n- UserName: " +  UserName + " \n- Door: " + transform("MAP", "ACDoorName.map", DoorID))
  return;
}

if(authDoors.state.toString.split(",").get(2).contains(DoorID) || authDoors.state.toString.split(",").get(3).contains(DoorID) || authDoors.state.toString.split(",").get(4).contains(DoorID)){
  logInfo("System","Security - Information: PERMIT ACCESS - " + transform("MAP", "ACDoorName.map", DoorID) + " wird geöffnet! - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)
  return;
}
else {
  logInfo("System","Security - Warning: ACCESS DENIED  - User not authorized - " + transform("MAP", "ACDoorName.map", DoorID) + "  - UID: " + UserID + " - UserName: " +  UserName + " - DoorID: " + DoorID)
  telegramActionSec.sendTelegram("Security - Warning: \nACCESS DENIED - User not authorized \n- UID: " + UserID + " \n- UserName: " +  UserName + " \n- Door: " + transform("MAP", "ACDoorName.map", DoorID))
  return;
}

It depends on your requirements for who and how the map between authorized users and doors is maintained. Do you need to manage them from the UI? Does some one else need to manage this list or just you?

The big problem with using and Item is that by default even non-authorized users can access, update, and command Items through the REST API. That’s why it’s little better than leaving a key under the welcome mat; if someone knows where to look they can unlock the door whether or not they are authorized to do so.

So you need to take both the authorization mapping and the unlocking of the door outside of Items. Or I believe you can limit the access to the REST API to only logged in users but then all your users will have to have accounts and log ins which can be troublesome in some circumstances. But if you do require the users to log in, those users can still access the REST API and give themselves permission to open a door they otherwise would not be allowed to.

So, for a more secure implementation you’d need to make it so the only way openHAB can unlock a lock is via a Rule without using Items. If this is not possible given the technology you might need to run a separate instance of OH and configure the firewalls and networking such that only your main OH instance can talk to it. Then you can issue a REST API call to that instance using sendHttpPutRequest or a call out to curl using executeCommandLine or the line in the rule. That would make it so the only way to unlock the door is through that rule (NOTE: this is really an unjustifiable statement and it requires a good deal of details about the specific technologies involved to ensure that it’s true).

Now we need to make sure that the mapping between users and doors cannot be modified through OH’s REST API except by a logged in admin user as well. There are all sorts of ways to accomplish that:

  • hard code it in the rule with a HashMap
  • use the Map transformation similar to your new version’s method for getting the door’s name
  • write it in a file (e.g. a Java .properties file, csv file, etc) and load and parse that file when the rule runs
  • outsource the mapping to a separate program that your rule calls

I’m sure there are more.

But, and I want to be very clear on this, you have to assess for yourself the actual risk and determine whether doing through all of this effort is going to cost less than the actual risk. If you live in a low crime area or have users who are unlikely to want to try to bypass the system then maybe the answer is no. If all your users will really have access to all doors anyway then the risk is low. Risk is usually represented with the following equation:

Risk = Likelihood * Impact

where Likelihood is a percentage and Impact is often expressed as a monetary value. Remember your time has value as well. But also remember risk has to be constantly reassessed as the Likelihood and Impact often change over time.

There are all sorts of ways to calculate Likelihood and Impact but often it comes down to making an educated guess, and that’s fine here.

The only comment I have about the new rule is the second and third return; statements are unnecessary. It looks a lot cleaner now and avoids the original problem nicely.

@rlkoshak Thanks you, now i must rethink all my other rules :joy: :see_no_evil:

The risk is minimal, the doors are in a wall / fence around the property. if u like u can jump over this. the system are only for usability, and i dont need a normal key for each user, it is easier to remove a coin out of the OH as to change als door locks. the access to the User Item in OH is only possible for 2 people, that risk is minimal. And the realy important door, the front door, they has an oldschool none smart lock.