More AAA refactoring

- parameterize the UDP port of the RADIUS server - clean up some unused constants - refactor config code to reduce repetitive code Change-Id: I797dabd12c8ccd522f1ab0812c2b03da7264f2c0

More AAA refactoring
- parameterize the UDP port of the RADIUS server - clean up some unused constants - refactor config code to reduce repetitive code Change-Id: I797dabd12c8ccd522f1ab0812c2b03da7264f2c0
Ray Milkey · Gerrit Code Review
Commit af86fbff625d5e2a8466f5248edfe7d3dd362585 af86fbff 1 parent c7c1d35a
Showing 3 changed files with 58 additions and 51 deletions
apps/aaa/src/main/java/org/onosproject/aaa/AAA.java
apps/aaa/src/main/java/org/onosproject/aaa/AAAConfig.java
apps/aaa/src/test/java/org/onosproject/aaa/AAATest.java
--- a/apps/aaa/src/main/java/org/onosproject/aaa/AAA.java
View file @af86fbf
+++ b/apps/aaa/src/main/java/org/onosproject/aaa/AAA.java
View file @af86fbf
@@ -112,6 +112,9 @@ public class AAA {
     // RADIUS port number
     protected long radiusPort;
+    // RADIUS server TCP port number
+    protected short radiusServerPort;
+
     // our application-specific event handler
     private ReactivePacketProcessor processor = new ReactivePacketProcessor();
@@ -206,8 +209,8 @@ public class AAA {
         TrafficSelector radSelector = DefaultTrafficSelector.builder()
                 .matchEthType(EthType.EtherType.IPV4.ethType().toShort())
                 .matchIPProtocol(IPv4.PROTOCOL_UDP)
-                .matchUdpDst(TpPort.tpPort(1812))
+                .matchUdpDst(TpPort.tpPort(radiusServerPort))
-                .matchUdpSrc(TpPort.tpPort(1812))
+                .matchUdpSrc(TpPort.tpPort(radiusServerPort))
                 .build();
         packetService.requestPackets(radSelector, CONTROL, appId);
     }
@@ -223,8 +226,8 @@ public class AAA {
         TrafficSelector radSelector = DefaultTrafficSelector.builder()
                 .matchEthType(EthType.EtherType.IPV4.ethType().toShort())
                 .matchIPProtocol(IPv4.PROTOCOL_UDP)
-                .matchUdpDst(TpPort.tpPort(1812))
+                .matchUdpDst(TpPort.tpPort(radiusServerPort))
-                .matchUdpSrc(TpPort.tpPort(1812))
+                .matchUdpSrc(TpPort.tpPort(radiusServerPort))
                 .build();
         packetService.cancelPackets(radSelector, CONTROL, appId);
     }
@@ -452,8 +455,8 @@ public class AAA {
             IPv4 ip4Packet = new IPv4();
             Ethernet ethPkt = new Ethernet();
             radiusMessage.setParent(udp);
-            udp.setDestinationPort((short) 1812);
+            udp.setDestinationPort(radiusServerPort);
-            udp.setSourcePort((short) 1812); // TODO: make this configurable
+            udp.setSourcePort(radiusServerPort);
             udp.setPayload(radiusMessage);
             udp.setParent(ip4Packet);
             ip4Packet.setSourceAddress(AAA.this.nasIpAddress.getHostAddress());
@@ -524,15 +527,9 @@ public class AAA {
             if (newCfg.radiusPort() != -1) {
                 radiusPort = newCfg.radiusPort();
             }
-
+            if (newCfg.radiusServerUDPPort() != -1) {
-            log.info("AAA app configuration:");
+                radiusServerPort = newCfg.radiusServerUDPPort();
-            log.info("NAS IP is {}", nasIpAddress);
+            }
-            log.info("RADIUS IP is {}", radiusIpAddress);
-            log.info("NAS MAC is {}", nasMacAddress);
-            log.info("RADIUS MAC is {}", radiusMacAddress);
-            log.info("RADIUS secret is {}", radiusSecret);
-            log.info("RADIUS switch is {}", radiusSwitch);
-            log.info("RADIUS port is {}", radiusPort);
         }
         @Override
--- a/apps/aaa/src/main/java/org/onosproject/aaa/AAAConfig.java
View file @af86fbf
+++ b/apps/aaa/src/main/java/org/onosproject/aaa/AAAConfig.java
View file @af86fbf
@@ -28,6 +28,7 @@ import org.onosproject.net.config.basics.BasicElementConfig;
 public class AAAConfig extends Config<ApplicationId> {
     private static final String RADIUS_IP = "radiusIp";
+    private static final String RADIUS_SERVER_PORT = "1812";
     private static final String RADIUS_MAC = "radiusMac";
     private static final String NAS_IP = "nasIp";
     private static final String NAS_MAC = "nasMac";
@@ -47,9 +48,6 @@ public class AAAConfig extends Config<ApplicationId> {
     // NAS MAC address
     protected static final String DEFAULT_NAS_MAC = "00:00:00:00:10:01";
-    // RADIUS uplink port
-    protected static final int DEFAULT_RADIUS_UPLINK = 2;
-
     // RADIUS server shared secret
     protected static final String DEFAULT_RADIUS_SECRET = "ONOSecret";
@@ -59,6 +57,24 @@ public class AAAConfig extends Config<ApplicationId> {
     // Radius Port Number
     protected static final String DEFAULT_RADIUS_PORT = "129";
+    // Radius Server UDP Port Number
+    protected static final String DEFAULT_RADIUS_SERVER_PORT = "1812";
+
+    /**
+     * Gets the value of a string property, protecting for an empty
+     * JSON object.
+     *
+     * @param name name of the property
+     * @param defaultValue default value if none has been specified
+     * @return String value if one os found, default value otherwise
+     */
+    private String getStringProperty(String name, String defaultValue) {
+        if (object == null) {
+            return defaultValue;
+        }
+        return get(name, defaultValue);
+    }
+
     /**
      * Returns the NAS ip.
      *
@@ -66,10 +82,7 @@ public class AAAConfig extends Config<ApplicationId> {
      */
     public InetAddress nasIp() {
         try {
-            if (object == null) {
+            return InetAddress.getByName(getStringProperty(NAS_IP, DEFAULT_NAS_IP));
-                return InetAddress.getByName(DEFAULT_NAS_IP);
-            }
-            return InetAddress.getByName(get(NAS_IP, DEFAULT_NAS_IP));
         } catch (UnknownHostException e) {
             return null;
         }
@@ -92,10 +105,7 @@ public class AAAConfig extends Config<ApplicationId> {
      */
     public InetAddress radiusIp() {
         try {
-            if (object == null) {
+            return InetAddress.getByName(getStringProperty(RADIUS_IP, DEFAULT_RADIUS_IP));
-                return InetAddress.getByName(DEFAULT_RADIUS_IP);
-            }
-            return InetAddress.getByName(get(RADIUS_IP, DEFAULT_RADIUS_IP));
         } catch (UnknownHostException e) {
             return null;
         }
@@ -117,10 +127,7 @@ public class AAAConfig extends Config<ApplicationId> {
      * @return mac address or null if not set
      */
     public String radiusMac() {
-        if (object == null) {
+        return getStringProperty(RADIUS_MAC, DEFAULT_RADIUS_MAC);
-            return DEFAULT_RADIUS_MAC;
-        }
-        return get(RADIUS_MAC, DEFAULT_RADIUS_MAC);
     }
     /**
@@ -139,10 +146,7 @@ public class AAAConfig extends Config<ApplicationId> {
      * @return mac address or null if not set
      */
     public String nasMac() {
-        if (object == null) {
+        return getStringProperty(NAS_MAC, DEFAULT_NAS_MAC);
-            return DEFAULT_NAS_MAC;
-        }
-        return get(NAS_MAC, DEFAULT_NAS_MAC);
     }
     /**
@@ -161,10 +165,7 @@ public class AAAConfig extends Config<ApplicationId> {
      * @return radius secret or null if not set
      */
     public String radiusSecret() {
-        if (object == null) {
+        return getStringProperty(RADIUS_SECRET, DEFAULT_RADIUS_SECRET);
-            return DEFAULT_RADIUS_SECRET;
-        }
-        return get(RADIUS_SECRET, DEFAULT_RADIUS_SECRET);
     }
     /**
@@ -183,10 +184,7 @@ public class AAAConfig extends Config<ApplicationId> {
      * @return radius switch ID or null if not set
      */
     public String radiusSwitch() {
-        if (object == null) {
+        return getStringProperty(RADIUS_SWITCH, DEFAULT_RADIUS_SWITCH);
-            return DEFAULT_RADIUS_SWITCH;
-        }
-        return get(RADIUS_SWITCH, DEFAULT_RADIUS_SWITCH);
     }
     /**
@@ -205,10 +203,7 @@ public class AAAConfig extends Config<ApplicationId> {
      * @return radius port or null if not set
      */
     public long radiusPort() {
-        if (object == null) {
+        return Integer.parseInt(getStringProperty(RADIUS_PORT, DEFAULT_RADIUS_PORT));
-            return Integer.parseInt(DEFAULT_RADIUS_PORT);
-        }
-        return Integer.parseInt(get(RADIUS_PORT, "-1"));
     }
     /**
@@ -221,4 +216,24 @@ public class AAAConfig extends Config<ApplicationId> {
         return (BasicElementConfig) setOrClear(RADIUS_PORT, port);
     }
+    /**
+     * Returns the RADIUS server UDP port.
+     *
+     * @return radius server UDP port.
+     */
+    public short radiusServerUDPPort() {
+        return Short.parseShort(getStringProperty(RADIUS_SERVER_PORT,
+                                                  DEFAULT_RADIUS_SERVER_PORT));
+    }
+
+    /**
+     * Sets the RADIUS port.
+     *
+     * @param port new RADIUS UDP port; -1 to clear
+     * @return self
+     */
+    public BasicElementConfig radiusServerUDPPort(short port) {
+        return (BasicElementConfig) setOrClear(RADIUS_SERVER_PORT, (long) port);
+    }
+
 }
--- a/apps/aaa/src/test/java/org/onosproject/aaa/AAATest.java
View file @af86fbf
+++ b/apps/aaa/src/test/java/org/onosproject/aaa/AAATest.java
View file @af86fbf
@@ -495,11 +495,6 @@ public class AAATest {
         assertThat(stateMachine.state(), is(StateMachine.STATE_AUTHORIZED));
     }
-
-    private static final String RADIUS_SECRET = "radiusSecret";
-    private static final String RADIUS_SWITCH = "radiusSwitch";
-    private static final String RADIUS_PORT = "radiusPort";
-
     /**
      * Tests the default configuration.
      */