KAFKA-4541: Support for delegation token mechanism #3616

omkreddy · 2017-08-03T15:28:41Z

Add capability to create delegation token
Add authentication based on delegation token.
Add capability to renew/expire delegation tokens.
Add units tests and integration tests

omkreddy · 2017-08-03T15:30:52Z

This is PR is a working version of delegation token implementation.

KIP: https://cwiki.apache.org/confluence/display/KAFKA/KIP-48+Delegation+token+support+for+Kafka

work done:

capability to create/renew/expire delegation tokens
zookeeper based token Storage and token caching
token Based authentication using scram.
command-line to manage tokens. This will be migrated to new Admin API ( will raise a KIP)

Minor deviations from the KIP:

tokenId field changed to string from int.
added owner, maxtime fields to CreateTokenResponse. This change is to make the response format similar to DescribeResponse
In KIP, We proposed to store scram credentials to ZK. But this is not required. Since server can create token hmac anytime, scram credentials can be created online during token creation and token retrieval from ZK.

Ongoing Work:

Support for token management in Admin API (will raise a KIP)
System test

@junrao @harshach @ijuma @rajinisivaram
Pls take a look whenever you get a chance.

asfgit · 2017-08-03T16:29:32Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/6518/
Test PASSed (JDK 7 and Scala 2.11).

asfgit · 2017-08-03T16:52:09Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/6504/
Test PASSed (JDK 8 and Scala 2.12).

asfgit · 2017-08-04T14:09:21Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/6569/
Test PASSed (JDK 7 and Scala 2.11).

asfgit · 2017-08-04T14:10:10Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/6554/
Test PASSed (JDK 8 and Scala 2.12).

asfgit · 2017-08-04T20:13:20Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/6585/
Test PASSed (JDK 7 and Scala 2.11).

asfgit · 2017-08-04T20:29:29Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/6570/
Test PASSed (JDK 8 and Scala 2.12).

asfgit · 2017-08-06T20:33:19Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/6597/
Test PASSed (JDK 7 and Scala 2.11).

asfgit · 2017-08-06T20:56:54Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/6582/
Test PASSed (JDK 8 and Scala 2.12).

omkreddy · 2017-09-06T07:00:57Z

@junrao @rajinisivaram it will be great if you can give your initial thoughts on implementation and approach.

junrao

@omkreddy : Thanks for the patch. Some comments below. Also, (1) could you add the new admin methods to org.apache.kafka.clients.admin.AdminClient since we will be phasing out kafka.admin.AdminClient? (2) could you add all the new ZK accesses to use the new ZookeeperClient?

junrao · 2017-09-14T00:44:39Z

clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java

+    //Token Create request
+    public static final Schema TOKEN_CREATE_REQUEST_V0 = new Schema(
+            new Field("renewers", new ArrayOf(STRING),
+                    "An array of token renewers. Renewer is an Kafka PrincipalType+name string, who is allowed to renew this token before the max lifetime expires."),


Should PrincipalType+name be in one field or two separate fields?

we can add two separate fields. updated the PR.

junrao · 2017-09-14T00:52:46Z

core/src/main/scala/kafka/server/KafkaApis.scala

+    }
+
+    val isTokenAuthenticated = request.session.principal.tokenAuthenticated
+    if (request.context.securityProtocol == SecurityProtocol.PLAINTEXT || isTokenAuthenticated)


Hmm, SSL can be used for just one-way authentication. In that case, the client is not authenticated by the server. So, we probably want to disallow token creation?

Ok..I have excluded SSL authentication also. we can review and enable if required in future

junrao · 2017-09-14T00:57:32Z

core/src/main/scala/kafka/server/KafkaApis.scala

+
+    val isTokenAuthenticated = request.session.principal.tokenAuthenticated
+    if (request.context.securityProtocol == SecurityProtocol.PLAINTEXT || isTokenAuthenticated)
+      sendResponseCallback( Errors.TOKEN_REQUEST_NOT_ALLOWED, -1)


It will be useful to define a meaningful constant for -1.

junrao · 2017-09-14T23:33:04Z

core/src/main/scala/kafka/server/TokenManager.scala

+  def startup() = {
+    if (config.tokenAuthEnabled) {
+      zkUtils.makeSurePersistentPathExists(ZkUtils.TokenInfoPath)
+      loadCache


ZkNodeChangeNotificationListener already has the logic to read all existing data from ZK during init(). So, calling loadCache() seems unnecessary here.

ZkNodeChangeNotificationListener only loads the change notifications. So we need loadCache to build tokenCache

junrao · 2017-09-14T23:45:23Z

core/src/main/scala/kafka/server/TokenManager.scala

+
+  import TokenManager._
+
+  type createResponseCallback = CreateTokenResult => Unit


Capitalize the type name?

junrao · 2017-10-24T00:25:41Z

clients/src/main/java/org/apache/kafka/common/security/token/TokenCache.java

+        //Cache to hold all the tokens
+        tokenCache = credentialCache.createCache(TOKEN_CACHE, TokenInformation.class);
+        //Cache to hold tokenId->hmac mapping. This is required for renew, expire requests
+        hmacIDCache = credentialCache.createCache(TOKEN_HMAC_ID_CACHE, String.class);


CredentialCache is really used for caching different Scram mechanisms? So, it's a bit weird to reuse that for storing tokens and hmacID? Could we just create two specific HashMap, one for token and the other for hmacID?

junrao · 2017-10-24T00:43:04Z

core/src/main/scala/kafka/server/TokenManager.scala

+    //Update Scram Credentials
+    val scramConfig = new Properties
+    prepareScramCredentials(base64Pwd, scramConfig)
+    tokenCache.updateCredentials(tokenInfo.tokenId, scramConfig)


It's a bit weird for scramConfig to use Properties here. Could we just use a HashMap?

junrao · 2017-10-24T00:44:56Z

core/src/main/scala/kafka/server/TokenManager.scala

+    tokenCache.updateCredentials(tokenInfo.tokenId, scramConfig)
+
+    //Update hmac-id cache
+    tokenCache.addHmacIdMapping(base64Pwd, tokenInfo.tokenId)


Is it possible to move all the three updates to inside TokenCache?

junrao · 2017-10-24T01:03:36Z

clients/src/main/java/org/apache/kafka/common/requests/RenewTokenRequest.java

+
+    public static RenewTokenRequest parse(ByteBuffer buffer, short version) {
+        return new RenewTokenRequest(ApiKeys.RENEW_TOKEN.parseRequest(version, buffer),
+            (short) version);


(short) is unnecessary.

junrao · 2017-10-24T01:34:21Z

core/src/main/scala/kafka/server/DynamicConfigManager.scala

@@ -38,6 +38,7 @@ object ConfigType {
  val Client = "clients"
  val User = "users"
  val Broker = "brokers"
+  val Token = "tokens"


Token doesn't seem to used. Is anything related to token configurable?

asfgit · 2017-11-16T19:22:05Z

FAILURE
No test results found.
--none--

asfgit · 2017-11-16T19:30:44Z

FAILURE
No test results found.
--none--

omkreddy · 2017-12-18T15:45:08Z

@junrao pinging for review

junrao

@omkreddy : Thanks for updated patch. Made another pass and the following are some comments.

junrao · 2018-01-09T23:50:54Z

core/src/main/scala/kafka/server/KafkaApis.scala

+  def allowTokenRequests(request: RequestChannel.Request) : Boolean = {
+    if (request.session.principal.tokenAuthenticated ||
+      request.context.securityProtocol == SecurityProtocol.PLAINTEXT ||
+      request.context.securityProtocol == SecurityProtocol.SSL)


It's actually useful to support 2-way SSL since configuring client side certificates in a shared environment such as Storm is still painful. Actually, in the case of 1-way SSL, the authenticated user will be ANONYMOUS. So, we can simply remove this check and allow tokens to be created for ANONYMOUS user.

junrao · 2018-01-10T00:03:24Z

clients/src/main/java/org/apache/kafka/common/requests/CreateTokenResponse.java

+
+    private static final Schema TOKEN_CREATE_RESPONSE_V0 = new Schema(
+        ERROR_CODE,
+        new Field("owner", STRING, "Token owner."),


To be consistent, this probably should be 2 separate fields of type and name like what's in CreateTokenRequest. Actually, is there a need to return owner? The client should know itself.

I was thinking it will be easy to construct DelegationToken object from response. I am not able to find easy way to get user principal on client side. For now, added 2 separate fields for owner field.

junrao · 2018-01-10T00:36:17Z

core/src/main/scala/kafka/server/TokenManager.scala

+  }
+
+  val tokenMaxLifetime: Long = config.delegationTokenMaxLifeMs
+  val tokenRenewInterval: Long = config.delegationTokenExpiryTimeMs


Sometimes we use time and some other times we use interval. It would be useful to make them consistent throughout the patch.

junrao · 2018-01-10T00:39:30Z

core/src/main/scala/kafka/server/TokenManager.scala

+        val tokenInfo = new TokenInformation(tokenId, owner, renewers.asJava)
+        tokenInfo.setIssueTimestamp(issueTimeStamp)
+        tokenInfo.setMaxTimestamp(maxLifeTimeStamp)
+        tokenInfo.setExpiryTimestamp(expiryTimeStamp)


Should we just pass in those 3 timestamps through the constructor?

junrao · 2018-01-10T00:48:03Z

core/src/main/scala/kafka/server/TokenManager.scala

+        } catch {
+          case e: Throwable =>
+            error("Exception while creating token", e)
+            responseCallback(CreateTokenResult(-1, -1, -1, "", Array[Byte](), Errors.forException(e)))


Is this for handling unexpected errors? If so, they are already handled in KafkaApis.handle().

junrao · 2018-01-11T01:42:53Z

core/src/main/scala/kafka/admin/DelegationTokenCommand.scala

+  def expireToken(adminClient: AdminClient, opts: DelegationTokenCommandOptions) = {
+    val hmac = opts.options.valueOf(opts.hmacOpt)
+    val expiryTimePeriodMs = opts.options.valueOf(opts.expiryTimePeriodOpt).longValue()
+    println("Calling token operation with hmac :" + hmac +" , expire-time-period : "+ expiryTimePeriodMs)


"Calling token operation" => "Calling expire token operation"

junrao · 2018-01-11T02:00:42Z

bin/kafka-delegation-tokens.sh

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+exec $(dirname $0)/kafka-run-class.sh kafka.admin.DelegationTokenCommand "$@"


Could we add the same script under bin/windows too?

junrao · 2018-01-11T02:15:52Z

core/src/test/scala/integration/kafka/api/TokenEndToEndAuthorizationTest.scala

+    for (_ <- 0 until producerCount)
+      producers += createNewProducer
+    for (_ <- 0 until consumerCount) {
+      consumers += createNewConsumer


Is there a test method that uses producers/consumers?

Yes. TokenEndToEndAuthorizationTest.scala extends SaslEndToEndAuthorizationTest. All the tests run with token authenticated clients.

junrao · 2018-01-11T02:17:35Z

core/src/main/scala/kafka/zk/ZkData.scala

@@ -534,3 +562,4 @@ object ZkData {
    } else ZooDefs.Ids.OPEN_ACL_UNSAFE.asScala
  }
 }
+


unneeded new line

junrao · 2018-01-11T02:18:33Z

core/src/main/scala/kafka/utils/ZkUtils.scala

@@ -68,6 +68,7 @@ object ZkUtils {
  val ConfigUsersPath = s"$ConfigPath/users"
  val ProducerIdBlockPath = "/latest_producer_id_block"

+


unneeded new line

ZKUtils changes are not required. reverted the changes.

omkreddy · 2018-01-11T18:03:26Z

@junrao Thanks for the review. Since adding new methods to org.apache.kafka.clients.admin.KafkaAdminClient requires KIP, I planning to do it separate PR. I will raise KIP and PR by next week. Updated the PR. Pls take a look.

junrao

@omkreddy Thanks for the latest patch. Just a few more minor comments.

junrao · 2018-01-12T01:13:52Z

core/src/test/scala/integration/kafka/api/TokenEndToEndAuthorizationTest.scala

+  override def initClients(producerSecurityProps: Properties, consumerSecurityProps: Properties) = {
+  }
+
+  def initTokenClients(token: DelegationToken) = {


Instead of overriding initTokenClients, would it be simpler to override clientSecurityProps() and put the additionally needed properties there?

modified the code to minimize the changes. Pls take a look

junrao · 2018-01-12T01:25:17Z

core/src/test/scala/unit/kafka/utils/JaasTestUtils.scala

@@ -72,14 +72,14 @@ object JaasTestUtils {

  case class ScramLoginModule(username: String,
                              password: String,
-                              debug: Boolean = false) extends JaasModule {
+                              debug: Boolean = false, tokenProps: Map[String, String] = Map.empty) extends JaasModule {


Could we put tokenProps in a new line?

junrao · 2018-01-12T01:25:56Z

core/src/test/scala/unit/kafka/utils/JaasTestUtils.scala


    def name = "org.apache.kafka.common.security.scram.ScramLoginModule"

    def entries: Map[String, String] = Map(
      "username" -> username,
      "password" -> password
-    )
+    ) ++ tokenProps.map { case (user, pass) => user -> pass }


Are the names user, pass appropriate? It seems that they are property name and value.

junrao · 2018-01-12T02:15:10Z

checkstyle/suppressions.xml

-
-    <suppress checks="JavaNCSS"
-              files="AbstractRequest.java"/>
+              files="Protocol.java|AbstractRequest.java|KerberosLogin.java|WorkerSinkTaskTest.java"/>


Protocol.java no longer exists.

junrao · 2018-01-12T02:20:39Z

core/src/test/scala/unit/kafka/utils/TestUtils.scala

@@ -158,11 +158,11 @@ object TestUtils extends Logging {
    enableSaslPlaintext: Boolean = false,
    enableSaslSsl: Boolean = false,
    rackInfo: Map[Int, String] = Map(),
-    logDirCount: Int = 1): Seq[Properties] = {
+    logDirCount: Int = 1, enableToken: Boolean = false): Seq[Properties] = {


Could we put enableToken in a new line?

junrao · 2018-01-12T02:29:35Z

core/src/main/scala/kafka/server/TokenManager.scala

+  def expireToken(principal: KafkaPrincipal,
+                  hmac: ByteBuffer,
+                  expireLifeTimeMs: Long,
+                  expireResponseCallback: RenewResponseCallback) {


Perhaps it's clearer to create a separate ExpireResponseCallback.

junrao · 2018-01-12T02:33:30Z

core/src/test/scala/unit/kafka/security/token/TokenManagerTest.scala

+    assertEquals(Errors.DELEGATION_TOKEN_EXPIRED, error)
+  }
+
+  def verifyCreateTokenResult(expected: CreateTokenResult): Unit = {


Perhaps it's better to define equal() in CreateTokenResult?

junrao · 2018-01-12T02:42:17Z

core/src/test/scala/unit/kafka/security/token/TokenManagerTest.scala

+    assert(expected.hmac sameElements createTokenResult.hmac)
+  }
+
+  def renewResponseCallback(ret: Errors, timeStamp: Long): Unit = {


Could this and a few other non-test methods be private?

junrao · 2018-01-12T02:45:47Z

core/src/test/scala/unit/kafka/security/token/TokenManagerTest.scala

+    val owner1 = SecurityUtils.parseKafkaPrincipal("User:owner1")
+    val owner2 = SecurityUtils.parseKafkaPrincipal("User:owner2")
+    val owner3 = SecurityUtils.parseKafkaPrincipal("User:owner2")
+    val owner4 = SecurityUtils.parseKafkaPrincipal("User:owner2")


The above 2 lines should use owner3 and owner4?

junrao · 2018-01-12T03:01:32Z

core/src/test/scala/unit/kafka/security/token/TokenManagerTest.scala

+      List()
+    }
+    else {
+      def eligible(token: TokenInformation): Boolean = {


Is it possible to avoid duplicating eligible()?

hmm..not found elegant way to do this. will try to check any other approach.

omkreddy · 2018-01-12T16:08:43Z

@junrao Thanks for the review. Updated the PR with review comments and some cleanups. Pls check.

soenkeliebau · 2018-01-12T16:20:18Z

core/src/main/scala/kafka/server/KafkaApis.scala

+  }
+
+  def allowTokenRequests(request: RequestChannel.Request) : Boolean = {
+    if (request.session.principal.tokenAuthenticated || request.context.securityProtocol == SecurityProtocol.PLAINTEXT)


Should we extend this to disallow requests on SSL listeners that don't enforce presenting a client certificate? Or rather, SSL endpoints with a user principal of ANONYMOUS - otherwise even authenticated users could't request a token on such an endpoint.

The inconsistency is that we disallow token creation on ANONYMOUS if security protocol is PLAINTEXT, but allow it if security protocol is SSL. We could probably also disable token creation if security protocol is SSL and user is ANONYMOUS.

junrao

@omkreddy : Thanks for updated patch. A few more minor comments below.

junrao · 2018-01-12T21:22:57Z

core/src/main/scala/kafka/server/DelegationTokenManager.scala

+                             tokenId: String,
+                             hmac: Array[Byte],
+                             error: Errors) {
+  override def equals(other: Any): Boolean = {


If we override equals(), we would want to override hashcode() as well.

junrao · 2018-01-12T21:39:42Z

core/src/test/scala/integration/kafka/api/DelegationTokenEndToEndAuthorizationTest.scala

+    super.setUp()
+  }
+
+  def getDelegationToken(): DelegationToken = {


Could we make this private? Also, perhaps it's more intuitive to name this createDelegationToken().

junrao · 2018-01-12T21:43:01Z

core/src/test/scala/integration/kafka/api/DelegationTokenEndToEndAuthorizationTest.scala

+    adminClient.close()
+
+    //wait for token to reach all the brokers
+    Thread.sleep(100)


This is probably not very reliable. Could we wait until the token is in every broker's token cache by poking into each server object?

junrao · 2018-01-12T21:45:02Z

core/src/test/scala/unit/kafka/security/token/delegation/DelegationTokenManagerTest.scala

@@ -15,7 +15,7 @@
  * limitations under the License.
  */

-package kafka.security.token
+package unit.kafka.security.token.delegation


Hmm, it seems we shouldn't include unit in package name.

junrao · 2018-01-12T22:18:12Z

core/src/main/scala/kafka/server/KafkaApis.scala

+      }
+      else {
+        val owners = if (describeTokenRequest.owners == null) null else describeTokenRequest.owners.asScala
+        def eligible(token: TokenInformation) : Boolean = {


Could we put eligible() in Object KafkaApis so that we can use it here and in the test?

@junrao Thanks for the review. made few changes to avoid the duplicate code.
Updated thePR. Pls take a look.

junrao · 2018-01-12T22:23:03Z

core/src/main/scala/kafka/server/KafkaApis.scala

+  }
+
+  def allowTokenRequests(request: RequestChannel.Request) : Boolean = {
+    if (request.session.principal.tokenAuthenticated || request.context.securityProtocol == SecurityProtocol.PLAINTEXT)


The inconsistency is that we disallow token creation on ANONYMOUS if security protocol is PLAINTEXT, but allow it if security protocol is SSL. We could probably also disable token creation if security protocol is SSL and user is ANONYMOUS.

…okens KAFKA-4542: DelegationToken based Authentication using SCRAM

omkreddy · 2018-01-16T15:19:31Z

KIP for admin client changes: https://cwiki.apache.org/confluence/display/KAFKA/KIP-249%3A+Add+Delegation+Token+Operations+to+KafkaAdminClient
PR is also ready is here. I planning to start discuss thread after merging this PR.

junrao · 2018-01-16T17:48:06Z

@omkreddy : Thanks for the PR. LGTM.

junrao · 2018-01-16T17:51:58Z

@omkreddy : Merged the patch to trunk. Be aware that the deadline for 1.1.0 KIP freeze is 01/23. So, ideally, it would be useful to get the admin client KIP in before that.

omkreddy · 2018-01-16T17:56:23Z

@junrao Thanks for your detailed review. I start discussion on admin client changes.

omkreddy force-pushed the KAFKA-4541 branch from 6f163e4 to d5f077c Compare August 4, 2017 13:08

omkreddy force-pushed the KAFKA-4541 branch from d5f077c to 2cc8c98 Compare August 4, 2017 19:15

omkreddy force-pushed the KAFKA-4541 branch from 2cc8c98 to 7d4efb1 Compare August 6, 2017 19:32

omkreddy force-pushed the KAFKA-4541 branch from 7d4efb1 to 1f28329 Compare August 21, 2017 19:06

omkreddy force-pushed the KAFKA-4541 branch from 1f28329 to 26ac4d6 Compare August 29, 2017 18:48

omkreddy force-pushed the KAFKA-4541 branch from 26ac4d6 to a3e5b46 Compare September 6, 2017 06:58

omkreddy force-pushed the KAFKA-4541 branch 4 times, most recently from 6917e25 to 27bbbd1 Compare September 19, 2017 10:06

omkreddy force-pushed the KAFKA-4541 branch 2 times, most recently from 54e85f8 to bdd62d4 Compare September 26, 2017 10:33

omkreddy force-pushed the KAFKA-4541 branch from bdd62d4 to 2cef5e7 Compare September 29, 2017 09:00

omkreddy force-pushed the KAFKA-4541 branch from 2cef5e7 to 29c009b Compare October 18, 2017 13:53

junrao reviewed Oct 25, 2017

View reviewed changes

omkreddy force-pushed the KAFKA-4541 branch from 29c009b to 1467d34 Compare October 26, 2017 09:36

omkreddy force-pushed the KAFKA-4541 branch from 1467d34 to 9b51ff3 Compare November 16, 2017 19:18

omkreddy force-pushed the KAFKA-4541 branch from 9b51ff3 to ae49551 Compare November 16, 2017 19:27

omkreddy force-pushed the KAFKA-4541 branch from 6ecb7a0 to 3a61eb3 Compare December 21, 2017 06:20

omkreddy force-pushed the KAFKA-4541 branch 2 times, most recently from d4385a7 to a2bfcb5 Compare January 4, 2018 16:16

junrao reviewed Jan 11, 2018

View reviewed changes

omkreddy force-pushed the KAFKA-4541 branch from a2bfcb5 to f29b41b Compare January 11, 2018 17:41

omkreddy force-pushed the KAFKA-4541 branch 2 times, most recently from 6a1a6e4 to f96e493 Compare January 11, 2018 20:08

junrao reviewed Jan 12, 2018

View reviewed changes

omkreddy force-pushed the KAFKA-4541 branch 2 times, most recently from e967dba to 86a05a2 Compare January 12, 2018 12:22

soenkeliebau reviewed Jan 12, 2018

View reviewed changes

junrao reviewed Jan 12, 2018

View reviewed changes

omkreddy added 3 commits January 13, 2018 05:56

KAFKA-4541: Add ability to create/renew/expire/describe delegation t…

bee9b2b

…okens KAFKA-4542: DelegationToken based Authentication using SCRAM

Address Jun's review comments

a90c299

Address Jun's comments

1685c67

omkreddy force-pushed the KAFKA-4541 branch 3 times, most recently from d560b22 to a464102 Compare January 13, 2018 03:14

Address Jun's review comments

31a0457

omkreddy force-pushed the KAFKA-4541 branch from a464102 to 31a0457 Compare January 13, 2018 03:24

junrao closed this in 27a8d0f Jan 16, 2018

omkreddy deleted the KAFKA-4541 branch July 3, 2018 15:46


		import TokenManager._

		type createResponseCallback = CreateTokenResult => Unit

		@@ -68,6 +68,7 @@ object ZkUtils {
		val ConfigUsersPath = s"$ConfigPath/users"
		val ProducerIdBlockPath = "/latest_producer_id_block"

KAFKA-4541: Support for delegation token mechanism #3616

KAFKA-4541: Support for delegation token mechanism #3616

Conversation

omkreddy commented Aug 3, 2017 • edited

omkreddy commented Aug 3, 2017 • edited

asfgit commented Aug 3, 2017

asfgit commented Aug 3, 2017

asfgit commented Aug 4, 2017

asfgit commented Aug 4, 2017

asfgit commented Aug 4, 2017

asfgit commented Aug 4, 2017

asfgit commented Aug 6, 2017

asfgit commented Aug 6, 2017

omkreddy commented Sep 6, 2017

junrao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asfgit commented Nov 16, 2017

asfgit commented Nov 16, 2017

omkreddy commented Dec 18, 2017

junrao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omkreddy Jan 11, 2018 • edited

Choose a reason for hiding this comment

omkreddy commented Jan 11, 2018

junrao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omkreddy commented Jan 12, 2018

soenkeliebau Jan 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junrao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omkreddy commented Jan 16, 2018

junrao commented Jan 16, 2018

junrao commented Jan 16, 2018

omkreddy commented Jan 16, 2018

omkreddy commented Aug 3, 2017 •

edited

omkreddy commented Aug 3, 2017 •

edited

omkreddy Jan 11, 2018 •

edited

soenkeliebau Jan 12, 2018 •

edited